CODEWITHSUNDEEP
clinuxgcclinked-list
Noir
Noir

Reputation: 484

How to print members of linked list correctly

I'm working with a linked list to get a list of all files from a directory recursively.

But for some reason I can not walk through the list to print the name of all nodes. The output of the line printf("%s\n", p->name); is just garbage. At compilation there are also two warnings. I marked the lines with a comment in the code below.

Here's the code:

typedef struct {
    char *name;
    struct FileList *next;
} FileList;

FileList *head = NULL;
FileList *cur = NULL;
long int totalSize = 0;

FileList* appendToFileList(char *name) {
    FileList *ptr = (FileList*) malloc(sizeof(FileList));

    if (ptr != NULL) {
        ptr->name = name;
        ptr->next = NULL;
        if (head == NULL) { //if its the first value add it to the head node
            head = cur = ptr;
            printf("added value to head %s\n", head->name); //this statement works correctly
        } else {
            cur->next = ptr; //warning "warning: assignment from incompatible pointer type" here
            cur = ptr;
            printf("added value %s\n", cur->name); //this statement works correctly
        }
    } else {
        return NULL;
    }
    return ptr;
}

int ftwCallback(char *file, struct stat *info, int flag) {
    if(flag == FTW_F) { //if entry is a file
        appendToFileList(file);
        totalSize += info->st_size;
    }
    return 0;
}

int main(int argc, char **argv) {
    //this function walks a given directory recursively and calls "ftwcallback" for each entry
    ftw(argv[1], ftwCallback, getdtablesize() - 1);
    FileList *p = head;

    while (p->next != NULL) {
        printf("%s\n", p->name); //just garbage here
        p = p->next; //warning "warning: assignment from incompatible pointer type" here
    }
    printf("Total size: %d\n", totalSize);

    return 0;
}

I adopted a bit of the code from this example.

What's going wrong here?

Upvotes: 0

Views: 148

Answers (3)

Ganesh
Ganesh

Reputation: 5990

You need to perform a strcpy instead of assignment as below

old code

ptr->name = name;

new code

ptr->name = malloc(strlen(name)+1);
strcpy(ptr->name, name);

In the old code, name is a temporary pointer on the stack which contains the string. Hence, this pointer will potentially get a new value for every new invocation of the function appendToFileList. Hence, when you try to print the ptr->name, it is pointing to an invalid location. Hence, this warrants the allocation of memory and subsequent copy as suggested in the new code.

Upvotes: 1

SJuan76
SJuan76

Reputation: 24895

The issue is not with printing, but with storing the value.

You just copy the value that is passed to your appendToFileList method. That char * points to a region of memory that gets reused after its scope is lost. malloc a memory region of size enough and strcpy the file name to it, and store a pointer to the "malloced" region.

Upvotes: 0

unwind
unwind

Reputation: 399959

This is because you never allocate memory for the name, you just retain the caller's pointer.

If the caller's pointer ceases to be valid, your copy does too.

You must store the string inside the node, or at least allocate memory dynamically to do so.

Also, please don't cast the return value of malloc() in C.

Upvotes: 1

Related Questions