CODEWITHSUNDEEP
cpointersmallocstructure
ssg
ssg

Reputation: 347

Value of structure variable is not printing as expected in pointer operation

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

typedef struct type {
    int a;
    char b[10];
}MyType;

MyType* p;
void appenddata(MyType* p);
void cleanup(MyType* pointer);

void mycode() {
  p = (MyType *)malloc(sizeof(MyType));

  if(p != NULL)
  {
    p->a = 10;
    strcpy(p->b, "sample");
    printf("p->a - %d\n",p->a);
    printf("p.b - %s\n",p->b);
  }

  appenddata(p);

  /* set the values for p and do some stuff with it */
  cleanup(p);
  if(p != NULL)
  {
    printf("p->a - %d\n",p->a); << Why value I am getting is '0' here and 
    printf("p.b - %s\n",p->b);  << No value is printed here
  }

}

void appenddata(MyType* p)
{
  if(p != NULL)
  {
    p->a = 10;
    strcpy(p->b, "sample appenddata");
    printf("p->a - %d\n",p->a);
    printf("p.b - %s\n",p->b);
  }
  else
  {
      printf("p is null\n");
  }
}

void cleanup(MyType* pointer) {
  free(pointer);
  pointer = NULL;
}

int main()
{
    printf("Hello, World!\n");
    mycode();
    return 0;
}

Output:

Hello, World!
p->a - 10
p.b - sample
p->a - 10
p.b - sample appenddata
p->a - 0
p.b -

In above program why I am getting '0' and no data displayed in mycode function, it should be '10'and 'sample' should be printed, whats wrong with the program?, as far as I knows when we do cleanup then I am not passing as reference so it should not affect local copy of structure. Please clarify my doubt

Upvotes: 1

Views: 368

Answers (4)

H.S.
H.S.

Reputation: 12679

Your program is having couple of undefined behavior.

First undefined behavior:

In the struct type, you have member b as an array of 10 characters:

char b[10];

And in the function appenddata(), you are copying a string of length more than 10 characters to b:

strcpy(p->b, "sample appenddata");
              ^^^^^^^^^^^^^^^^^

Accessing array out of bounds is undefined behavior.

To resolve this increase the size of array b, like b[50], and use strncpy which gives you some control over the number of characters to be copied to the destination. Read about strncpy thoroughly before using it.

Second undefined behavior:

When you free a dynamically allocated memory block, its lifetime is over.

From C Standard#6.2.4p2

The lifetime of an object is the portion of program execution during which storage is guaranteed to be reserved for it. An object exists, has a constant address,33) and retains its last-stored value throughout its lifetime.34) If an object is referred to outside of its lifetime, the behavior is undefined. The value of a pointer becomes indeterminate when the object it points to (or just past) reaches the end of its lifetime. [emphasis mine]

Here, you are accessing the p after freeing it:

  if(p != NULL)
  {
    printf("p->a - %d\n",p->a); << Why value I am getting is '0' here and 
    printf("p.b - %s\n",p->b);  << No value is printed here
  }

which leads to undefined behavior. This is occurring because of the if condition p != NULL which will be true even after calling freeing p in cleanup(). In the cleanup() function, you are freeing pointer p but assigning NULL to local variable pointer of type MyType *. Instead, you should do p = NULL; after calling cleanup() function.

Upvotes: 2

ntshetty
ntshetty

Reputation: 1305

To avoid warnings related to free add #include<stdlib.h> to source, in cleanup you cleaning the structure data and de-referencing the local pointer instead of main pointer. hence the main pointer becoming dangling pointer.

Added some printf to demo:

Hello, World!
p->a - 10
p.b - sample
p->a - 10
p.b - sample appenddata
Pointer Address before cleanup : 0x14a5020
Pointer Address before free : 0x14a5020
Pointer Address after free : (nil)
Pointer Address after cleanup : 0x14a5020
p->a 1- 0
p.b 2-

Hence trying to print the garbage of dangling pointer pointed.

To avoid you can do either way.

Replace cleanup function as MACRO

#define cleanup(p) free(p); p=NULL

or return the pointer from cleanup funciton

MyType* cleanup(MyType* pointer);

....................
 /* set the values for p and do some stuff with it */
p = cleanup(p);
.......................

MyType* cleanup(MyType* pointer) {

  free(pointer);
  pointer = NULL;
  return pointer;
}

Upvotes: 0

Gerhardh
Gerhardh

Reputation: 12404

As far as I knows when we do cleanup then I am not passing as reference so it should not affect local copy of structure. Please clarify my doubt

You are partially right. In 'cleanup' you only have a copy of p. Assigning NULL to it does not change anything.

But you pass the value of global p to the function and then call free. This does not change p but nevertheless frees the memory where it is pointing to.

After calling cleanup you are not allowed to access that memory location again.

Your expectation to get

10
sample

is based on a wrong assumption. You could get any value as this is undefined bahaviour anyway.

BTW: If the memory wasn't invalid, you would get 

10
sample appenddata

This is due to the same reason.

In appenddata you don't change p but you change p->b which is still in place after you leave the function.

Upvotes: 1

user9432978
user9432978

Reputation:

I think that your problem is in cleanup(p) You shouldn't do it before printing data.

Upvotes: 0

Related Questions