-3

I have a char pointer in the structure to store names.When I inserted the values and printed the last value of name is getting printed for all nodes.

typedef struct tests{
      int id;
      char *p;
  struct tests *next;
}test;'

'void add(test **root,int id,char *name){
  test *newnode=(test* )malloc(sizeof(test));
  newnode->id=id;
  newnode->p=name;
  newnode->next=NULL;
  test *curr;
  curr=(*root);
  if((*root)==NULL){
    (*root)=newnode;
  }

  else{
    while(curr->next!=NULL){
      curr=curr->next;
    }
    curr->next=newnode;
  }
}
elango
  • 1
  • 2
  • 2
    `newnode->p=name;` change to like `newnode->p=strdup(name);` – BLUEPIXY Jun 26 '17 at 12:59
  • Do you intend to use a hard copy of the name or not? Because you don't allocate memory for `p`. What is `name` pointing at? Is it a string literal? Why isn't it const? – Lundin Jun 26 '17 at 12:59
  • Why do beginners almost always use a two-star pointer instead of simply returning the result? – too honest for this site Jun 26 '17 at 13:04
  • @Olaf maybe they are reserving the return value for an error int, ie 0 will mean no error, and higher values will index an array of error messages.. (OK, unlikely, but it's nicer than 'they just copy 'int myFunction' from net examples without knowing what it means). – ThingyWotsit Jun 26 '17 at 13:07
  • strdup is working.name is a string. – elango Jun 26 '17 at 13:10
  • Do you have a loop where you call `add`, passing an array as the last argument? Then all nodes will have a pointer to the same array (or rather the first element of the array). There's a reason why it's so important to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve) to show us. – Some programmer dude Jun 26 '17 at 13:10
  • @ThingyWotsit: Yes, that must be it. Because beginners always care about error checking and proper error status reporting … – too honest for this site Jun 26 '17 at 13:12

1 Answers1

0

Most likely what you intend to achieve is something along these lines:

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

typedef struct test_s {
  int id;
  char *p;
  struct test_s *next;
} test_t;

test_t** add(test_t **root, int id, char const *name){
  size_t len = strlen(name);
  test_t *newnode=(test_t*)malloc(sizeof(test_t)+len+1); // 1 goes for \0 terminator
  newnode->id=id;
  strcpy(newnode->p=(void*)(newnode+1), name);
  newnode->next=NULL;
  test_t **tail = root;
  while (*tail) {
    tail = &(*tail)->next;
  }
  *tail = newnode;
  return &(newnode->next);
}

Problem with your code is, you never copied your string assigning just a single pointer instead.

Take a look at pointers mechanics once again, additionally I would advise you to check string.h reference.

tgregory
  • 554
  • 3
  • 9