Failed to add a node to linked list

Question

I make the changes but I can't add more than 2 nodes its will freez but if 1 or 2 node will work well what is the reason??? I gave_up I can do nothing for that This is my code till time

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

struct info{
    int num;
    char name[15];
    struct info *next;
};

struct info *first,*current,*new_s;
int struct_num;
void add_struct(void);

int main(){
    first=NULL;
    add_struct();
    puts("done");
    add_struct();
    puts("done");
    add_struct();
    puts("done");

    return(0);
}

//struct add function

void add_struct(void){

new_s= malloc (sizeof(struct info));
if(!new_s){
    puts("error");
    exit (1);
}
if(first==NULL){
   first = current= new_s;
   first->next = NULL;
}else{
    current=first;

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

    current->next=new_s;
    current=new_s;
}

struct_num++;
}

Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, May 14 '15 at 07:47
using a [sentry node](http://pastebin.com/JAfq6ep1) helps you avoid all special cases, such as empty list, first node, last node etc — sp2danny, May 14 '15 at 08:20

score 5 · Accepted Answer · edited May 23 '17 at 12:06

The problem in your code is

if( first==NULL){
first->next=new_s;

if first is NULL, you should not dererefence it. It is logically wrong, and invokes undefined behaviour.

I think, what you want instead, is something like (pseudo-code)

if(first == NULL){
    first = new_s;
    first->next = NULL;

That said,

    current->next=new_s;
    current=new_s;

also looks problematic. The second statement there is wrong and not required, rather, you can add something like

   current->next = new_s;
   current->next->next = NULL;

Finally, your struct_num variable should be global, as per the current usage.

Note:

The recommended signature of main() is int main(void).
Please do not cast the return value of malloc() and family in C.
Always check for malloc() success before using the returned pointer.

i make the changes but when i try to make a list from 3 node or more its freez ,2 node work well, what is the reason ??? — technomacx, May 16 '15 at 21:07

Vlad from Moscow · Answer 2 · 2015-05-14T08:06:37.380

Function add_struct is wrong For example if first is equal to NULL then you may not write

    first->next=new_s;

Take into account that there is no any sense to declare local variable of the function struct_num because it is always destroyed after exiting the function and moreover it even is not initialized in the function.

int struct_num;

If you need a count of nodes in the list then place it outside the function.

The function itself can look the following way

int struct_num;

int add_struct( void )
{
    new_s = ( struct info * )malloc( sizeof( struct info ) );

    if ( new_s == NULL ) return 0;

    // initialize data members num and name of the allocated structure
    new_s->next = NULL;

    if ( first == NULL )
    {
        first = new_s;
    }
    else
    {
        current->next = new_s ;
    }

    current = new_s;
    ++struct_num;

    return 1;
}

Failed to add a node to linked list

2 Answers2