Segmentation fault on 6th insert in linked list

Question

i'm making a program where you can create a linked list and modify/delete/add/print value, but i'm getting some error testing it. First one is segmentation fault 11: when you add the 4th value and there are all equal values (1, 1, 1, 1), trying to print the list will get you a segfault, can't figure it out why. Second error (for the moment): sometimes the 6th or 8th value added will delete all other values, living the last two inserted and placing a 0 in tail.

This is the portion of code interested:

typedef struct list
{
    int value;
    struct list* next_ptr;
}tylist;

int main()
{

    tylist *ptrptr;
    ptrptr = NULL;
    printf("\nI'm going to initialize the list now\n");
    action(choicer(), &ptrptr);
    return 0; 
}   
void insert_at_beg(tylist** ptrptr, int value)
{
    if(*ptrptr != NULL)                             
    {
        tylist* tmp_ptr;
        tmp_ptr = *ptrptr;
        *ptrptr = (tylist*)malloc(sizeof(tylist));
        (*ptrptr)->value = value;
        (*ptrptr)->next_ptr = tmp_ptr;
        free(tmp_ptr);
    }
    else
    {
        printf("\nList is empty, that's your first entry\n");
        *ptrptr = (tylist*)malloc(sizeof(tylist));
        (*ptrptr)->value = value;
        (*ptrptr)->next_ptr = NULL;
    }
void action(int choice, tylist **ptrptr)
{
    switch(choice)
    {
    int value;
    case 1:
        printf("\nValue: ");
        scanf("%d", &value);
        insert_at_beg(ptrptr, value);
        action(choicer(), ptrptr);
        break;


    case 8:
        visit_list(ptrptr);
        action(choicer(), ptrptr);
        break;
    }
}
void visit_list(tylist **ptrptr) 
{
    while((*ptrptr) != NULL)
    {
        printf(" %d", (*ptrptr)->value);
        ptrptr = &((*ptrptr)->next_ptr);
    }
}

I've deleted some lines of code on purpose, because it seems just long enough this way, if it's unclear i'll post more.

P.s. i'm sorry for my bad english, i'm trying to improve it.

This *looks* like C, but either way, C != C++. You should generally only tag the language you are using/compiling. — crashmstr, Feb 17 '16 at 14:12
Please get rid of that menu/choice stuff. Create a small example with fixed input to reproduce the problem. (Also, `action` is repeatedly calling itself. That's not a sane way to implement a menu. Use a loop.) — M Oehm, Feb 17 '16 at 14:15
@Dario de cianni Show a compiled example The code you showed will not compile at least due to this declaration tylist *ptrptr; — Vlad from Moscow, Feb 17 '16 at 14:15
Also (perhaps a transcription error?) your `typedef` is `stylist`, but you use `tylist`. — crashmstr, Feb 17 '16 at 14:16
@crashmstr edited tag C++ and type stylist. @M Ohem should i use a while with a condition like while(response != 0)? Vlad from Moscow the code compile, otherwise how could i be posting this question? — iba, Feb 17 '16 at 14:25
@Dariodecianni the point was that the code *as posted* would not compile, and we can't read your mind or your screen. — crashmstr, Feb 17 '16 at 14:32

score 1 · Accepted Answer · edited May 23 '17 at 11:50

1

    tmp_ptr = (tylist *)malloc(sizeof(tylist));
    tmp_ptr = *ptrptr;

You just leaked sizeof( tylist ) in memory. (You edited this one out after I copied it.)

Oh, and don't cast the result of malloc() in C.

    (*ptrptr)->next_ptr = tmp_ptr;
    free(tmp_ptr);

Your next_ptr now points to free()'d memory. Any access to next_ptr from here on (as in visit_list()) is undefined behaviour.

int value;
case 1:
    printf("\nValue: ");
    scanf("%d", &value);

Never use scanf() for parsing user input. (Use fgets() and parse in-memory.)

If you insist on using scanf(), at least check the return code -- value might be uninitialized at this point if the user did enter a non-number.

    return 0; 
}

I don't see any code for releasing the memory once you are done with your list. Not free()ing allocated memory before you end your program is a memory leak. That e.g. Windows or Linux recover that memory as they tear down the program's process doesn't change the fact that your program leaked the memory.

edited May 23 '17 at 11:50

Community

1
1

answered Feb 17 '16 at 14:18

DevSolar

67,862
21
134
209

Now i see, but should I do it in another way or shouldn't i free it? And: why it's working for the first 4 insertion? – iba Feb 17 '16 at 14:26
@Dariodecianni: Well, with every new item you add to your list, you `free()` the previous one, but have the new item still point at the (now invalid) memory address of the previous one. What do you think happens in `visit_list()` when, after printing the `value` of the most recent item, your program follows the pointer-to-invalid-memory to get at the next item? It is [undefined behaviour](http://stackoverflow.com/questions/2397984) to reference invalid memory. Your program may crash the first time, or "work" a hundred times. It's a broken program, in any case. – DevSolar Feb 17 '16 at 14:32
1

thank you for the help, i apologise all you for making such a stupid/uncorrect question. – iba Feb 17 '16 at 21:05

Segmentation fault on 6th insert in linked list

1 Answers1