I am currently working on a project in which I need to add a Node at the end of a linked list (which should be easy nut is driving me nuts). The problem is that when I try to print the full linked list I get the data of the last node repeatedly. My code:

struct page *add_page (struct page *web, char *url){
    struct page *newNode = (struct page*) malloc(1*sizeof(struct page));
    newNode->url = url;
    newNode->next = NULL;
    
    if(web == NULL){
        web = newNode;
    }else{
        struct page *curr = web;
        while(curr->next != NULL){
            if(newNode->url==curr->url){
                printf("URL \"%s\" is already on the web\n",newNode->url);
                return web;
                free(newNode);
            }else{
                curr = curr->next;
            }
        }
        curr->next = newNode;
    }
    return web;
    free(newNode);
}

I am sure that the problem is on this function. I have tried other ways but this is the best I could get. Thanks in advance.

CodePudding user response：

You are probably using the same char pointer in your multiple calls add_page and only change the content that it points to to between calls. This means that you only have one char pointer, which is getting shared by all nodes you create. Any change to the string stored there, will be the string that all nodes point to.

You should either change the calling code, so that it creates a new string instead of mutating the existing one, or else make sure that the function takes a copy of the provided url string. Since you did not provide the calling code, I'll go for the second solution.

Some other issues:

• you should not compare char pointers to detect a duplicate node, but compare the string contents using strcmp.
• You have dead code following your return statements. In the first case, the free call should happen before the return statement. The final free however makes no sense in this function. That should happen later, at a moment you want to remove a node or the whole tree, not in this function.

struct page *add_page (struct page *web, char *url) {
    struct page *newNode = (struct page*) malloc(1*sizeof(struct page));
    // Store a copy of the url string:
    newNode->url = (char *) malloc(strlen(url) 1);
    strcpy(newNode->url, str);

    newNode->next = NULL;
    
    if (web == NULL) {
        web = newNode;
    } else {
        struct page *curr = web;
        while (curr->next != NULL) {
            // Compare the contents of the two strings
            if (strcmp(newNode->url, curr->url) == 0) {
                printf("URL \"%s\" is already on the web\n", newNode->url);
                // Free both the copied url and the new node itself
                free(newNode->url);
                free(newNode);
                // ...and only then return
                return web;
            } else {
                curr = curr->next;
            }
        }
        curr->next = newNode;
    }
    return web;
    // Remove dead code after this `return`
}

Don't forget to properly free the url before freeing the node whenever you need to remove a node.

CodePudding user response：

Well i ran on my compiler and didn't found any errors , it was working fine maybe there is something wrong with your show function or implementation of linked list, but i would like to make 1 suggestion , when you are checking the statement :

while(curr->next != NULL){
//Code
}

Make it ,

while(curr != NULL){
}

Because it then ignores your last node and search operation wouldn't be performed on it

and in the update statement of

curr = curr->next 

Modify it to

if(curr->next == NULL){
break;
}
curr = curr->next;

and keep your free statement before return as it will never be won't after return