0

Is there anything wrong with the following functions?. Somehow, they are creating a segment fault.

struct processNode* create_node()
{
    struct processNode* newNode =  (struct processNode*)malloc(sizeof(struct processNode));
    newNode->next = NULL;
    return newNode;
}

struct processNode* append_node(struct processNode* list,struct processNode* newNode )
{
    struct processNode* tracker= NULL;
    tracker = list;

    if(tracker == NULL)
    {
        tracker = newNode;
    }
    else
    {
        while(tracker->next != NULL)
        {
            tracker =tracker->next;
        }
        tracker->next = newNode;
        tracker = tracker->next;
    }

    tracker->next=NULL;
    tracker = list;
    return tracker;
}

I am creating a shell in C and need to create a link list to parse the command lines from the user. In the second function, I intend to return a new list with the new appended pointer;

Mark Tolonen
  • 166,664
  • 26
  • 169
  • 251
daniel
  • 557
  • 1
  • 4
  • 15
  • 1
    Consider `list` as NULL. Now walk that function. *Carefully*. What do you think `tracker = list` does (note: it appears *twice* in that function; once in the `if` block, one *after* the `else` block). Hmmm. – WhozCraig Apr 17 '17 at 06:11
  • Have you actually tried to run this in a debugger (as per the title of your question)? That should show you where things go wrong. I also find valgrind a useful tool for these issues, since it'll be likely a memory allocation gone wrong. –  Apr 17 '17 at 06:11
  • [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). – Sourav Ghosh Apr 17 '17 at 06:21

2 Answers2

1

I suppose the function shall return a pointer to the head of the list.

Assume the function is called with list being NULL

So this is fine:

    if(tracker == NULL)
    {
      tracker = newNode;
    }

but here

    tracker = list;   <---- Not good....
    return tracker;

You overwrite tracker and return NULL

You could try like:

struct processNode* append_node(struct processNode* list,struct processNode* newNode )
{
    struct processNode* tracker= NULL;
    tracker = list;

    if(tracker == NULL)
    {
        tracker = newNode;
        return tracker;        // Notice
    }

    while(tracker->next != NULL)
    {
        tracker =tracker->next;
    }
    tracker->next = newNode;

    return list; 
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
  • "you overwrite tracker and return null", umm how is that possible. List_head is the same pointer and tracker works on the same list_head is pointing to, so if I set tracker = list_head at the end, how is that a problem? – daniel Apr 17 '17 at 06:29
  • @Daniel `tracker` and `list` are **not** the same pointer. They are different pointers. Changing `tracker` does **not** change `list`. So if your function is called with `list` being `NULL`, the function returns `NULL` – Support Ukraine Apr 17 '17 at 06:37
0

The following cases will give you seg fault here on line tracker->next=NULL;:

  1. tracker != NULL && newNode == NULL
  2. tracker == NULL

        ....
    tracker = tracker->next;
    }
    
    //THE FOLLOWING LINE WILL CAUSE PROBLEM
    tracker->next=NULL; 
    tracker = list;
    return tracker;
     ....
    }
    

You can do like this:

struct processNode* append_node(struct processNode* list,struct processNode* newNode )
{
    struct processNode* tracker= NULL;
    tracker = list;

     if(!newNode){
        //Do nothing 
     }          
    else if(tracker == NULL)
    {
        tracker = newNode;
    }
    else
    {
        while(tracker->next != NULL)
        {
            tracker = tracker->next;
        }
        tracker->next = newNode;
        tracker = tracker->next;
    }

    return tracker;
}
Shridhar R Kulkarni
  • 6,653
  • 3
  • 37
  • 57