Struct becomes NULL even after assigning value from another function in a loop

Question

I am having troubles with my code. As you can see below, I have to make a list using linked list technique. My problem is that when I printf the s->item outside the add() function after performing it, the program crashes. When I print the s->item inside the add() function, it does print the correct data. Why does Statistician s become null again, even if the intialization of it is outside the while loop?

Please don't mind the typedefs, they are given by our instructor and we have to use it as basis so we dont have to change anything with the typedefs and structs.

typedef struct node *nodePtr;
struct node {
    int item;
    nodePtr next;
};
typedef nodePtr Statistician;

Statistician newStatistician(){
      Statistician s = (Statistician)malloc(sizeof(Statistician));
      s = NULL;
      return s;
}

void add(Statistician s, int x){

Statistician newNode = (Statistician)malloc(sizeof(Statistician));

if(s == NULL){ // first node
    printf("first");
    newNode->next = NULL;
    newNode->item = x;
    s = newNode;

main(){
int menuChoice, clearDataChoice, x, outputInt, exitChoice, check;
float  outputFloat;
Statistician s = newStatistician();

while (TRUE){
    printf("\t\t*** STATISTICIAN PROGRAM v1 ***\n\n\n");
        printf("Please enter data to be added : ");
        x = inputNum();
        add(s, x);
        printf("%d", s->item);
//... bunch of other code

   if(exitChoice==TRUE)
        return 0;
    else{
        printf("\n\nPress any key to continue...");
        getch();
        system("cls");
        fflush(stdin);
        continue;
    } 
}

EDIT: add and main are 2 different functions

   void add(Statistician s, int x){}
    main(){}

`malloc` returns a pointer, not an object. `Statistician s = (Statistician)malloc(sizeof(Statistician));` is wrong. See also https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — SergeyA, Mar 04 '19 at 21:46
You should include a [mcve] that demonstrates the problem. As it is, you have chunks of functions (`add` is cut off) lying around. Also, `newStatistician` leaks memory, and is always returning a NULL pointer. — Phil M, Mar 04 '19 at 22:07
Also, you're only allocating enough memory for a pointer, and not the full node object; you might want to remove the second typedef, as that appears to be causing some confusion. — Phil M, Mar 04 '19 at 22:12
Friendly advice: never typedef a pointer. You need to know when you're dealing with a pointer. Hiding the pointer with a typedef actually makes the code harder to read. For example, in the `newStatistician` function, `sizeof(Statistician)` is the size of a pointer, not the size of the node structure. — user3386109, Mar 04 '19 at 22:20
This code has some missing brackets and stuff, you need not cast the result of malloc, the while loop could also be changed to a do-while. Ignore what I said in my second comment. After you fix up your code a bit (according to what others have said ), try your add() with the dereference &. add(s&, x) — timi95, Mar 04 '19 at 22:29
@user3386109 what keyword do i use inside the malloc to make it have the size of the node structure? — German III Felisarta, Mar 04 '19 at 23:11
@timi95 if i use &s in add() it considers *&s* as a pointed variable of double pointer which *Statistician s* is not — German III Felisarta, Mar 04 '19 at 23:13
Among other problems, your `newStatistician` function always returns a null pointer. A bigger problem: The code in your question will not compile. Please update your question with a [mcve] (follow that link). — Keith Thompson, Mar 04 '19 at 23:22

dgnuff · Answer 1 · 2019-03-04T22:18:04.470

1

Talk to your instructor and ask him/her to explain the scope of variables to you.

The basic cause of the problem is that Statistician is a node *. So, you're passing a pointer to a Statistician to add(), but add() receives a private copy of the pointer itself: its variable s. You're modifying that private copy of s, but that modification doesn't affect the s in main(). So all the changes you make to the private copy in add() are lost when it returns.

The most usual way to solve this, and it's a fairly common pattern in C, is to return the possibly modified private copy of s from add(). So you'd change it's declaration to:

Statistician add(Statistician s, int x)

and then add the following at the very bottom:

return s;

Then when you actually call it in main() change the line to this:

s = add(s, x);

Give that a try and see how it works.

edited Mar 04 '19 at 22:18

answered Mar 04 '19 at 22:07

dgnuff

3,195
2
18
32

1

Bear in mind that `Statistician` is, it seems, already a pointer, due to the dodgy typedeffing. – Phil M Mar 04 '19 at 22:11
@PhilM Ugh. I had the prototype of `add()` all wrong. It's fixed now. Code like this tends to reinforce my agreement with Herb Sutter's Guru when she said that typedefs that just do pointer adornment are vanity, and lead to trouble. – dgnuff Mar 04 '19 at 22:20
Thanks for the quick reply, im gonna talk to my instructor about this. But it I think that I will be told to just follow the structure given – German III Felisarta Mar 04 '19 at 22:41
Why does Statistician s become a private variable if it is a pointer? – German III Felisarta Mar 04 '19 at 22:42
I mean, how does it not change the value of struct node if it is a pointer? – German III Felisarta Mar 04 '19 at 23:07
Because a pointer is just a value, like an `int` is. If you pass an int by value and change it inside the function, that change won't propagate outside the function, since you've only changed a copy. Same for the **value** of a pointer. That's different than changing the value of what the pointer points to. – Phil M Mar 04 '19 at 23:26

Struct becomes NULL even after assigning value from another function in a loop

1 Answers1