CODEWITHSUNDEEP
carrayspointers
etienz
etienz

Reputation: 101

Changing values in elements of an array of structs

I am working on an assignment and ran into challenging problem. As far as I'm concerned and from what I've learnt the code that follows should be correct however it does not work. Basically what I am trying to is copy a string value into the variable member of a structure the is part of an array passed into a method as a pointer. What am I missing?

typedef struct
{
  char * name; //variable in struct I am trying to access
} Struct;

void foo(Struct * arr) //array of Structs passed into function as a pointer 
{   
  int i = 0;
  while(i++ < 2)    
  {
      arr[i].name = malloc(sizeof(char *));  //assigning memory to variable in each Struct
      arr[i].name = strdup("name");  //copying "name" to variable in each Struct
      printf("C - %s\n", arr[i].name);  //printing out name variable in each Struct
  } 
}

main()
{
  Struct * arr;  //defining pointer
  arr = calloc(2, sizeof(Struct));  //allocating memory so pointer can hold 2 Structs
  foo(arr);  //calling function foo passing pointer into function   
  return 0;
}

This code compiles and runs however it does not do what it is designed to do. Forgive me if it is something trivial. I am new to the language C

Upvotes: 0

Views: 9168

Answers (3)

ryyker
ryyker

Reputation: 23208

This is not answering your question directly, but addresses an issue to big to put into a comment...
Additional issue: You probably did not intend to allocate only a (char *) worth of memory to a variable intended to hold at least "name". Change;

arr[i].name = malloc(sizeof(char *));

to:

arr[i].name = malloc(sizeof(char)*strlen("name")+1); //+1 for '\0'

or better yet, use char *name="name";, then:

arr[i].name = malloc(sizeof(char)*strlen(name)+1);

Even more general (and better): 

char *name;

name = malloc(strlen(someInputString)+1);  
//do stuff with name... 
free(name);

Now, you can allocate name to any length needed based on the length of someInputString.

[EDIT] Etienz, I wanted to address one more thing, alluded to by @H2CO3 above, but not really explained, that I think might be useful to you:
Regarding your desire to have room for two structs, because you typedef'd your struct, you can simply do something like this: (but I am going to change the name you used from Struct to NAME :) The whole point being that when a struct is created as an array, you do not need to use calloc or malloc to create space for them, it is done as shown below...

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

typedef struct{
    char *name;
}NAME;
//use new variable type NAME to create global variables:
NAME n[2], *pN; //2 copies AND pointer created here
//prototype func
int func(NAME *a);
int main()
{
    pN = &n[0];  //pointer initialized here
    func(pN);    //pointer used here (no malloc or calloc)
    printf("name1 is %s\nname 2 is %s", pN[0].name, pN[1].name);
    return 0;
}

int func(NAME *a)
{
    char namme1[]="andrew";
    char namme2[]="billebong";
    //You DO have to allocate the members though
    a[0].name = malloc(strlen(namme1)+1);
    a[1].name = malloc(strlen(namme2)+1);

    strcpy(a[0].name, namme1);
    strcpy(a[1].name, namme2);
    return 0;
}

Upvotes: -1

fkl
fkl

Reputation: 5535

Extending on 2 pointed out correctly already,

arr[i].name = strdup("name");

Even if you use following instead of above,

strcpy(array[i].name, "name");

you haven't allocated enough bytes to store the string i.e. this is wrong

arr[i].name = malloc(sizeof(char *));
// even if pointer is 8 byte here, concept isn't right

Should be something like

arr[i].name = malloc(strlen("name")+1);
// or MAX_SIZE where it is greater than the possible "name".

Or better yet, remove the malloc at all, strdup takes care of allocation itself

Upvotes: 1

yan
yan

Reputation: 20982

Two issues:

  1. while(i++ < 2) This line changes the value of i as soon as it checks it, so your loop body will not be the same as it was checked.
  2. arr[i].name = strdup("name"); overwrites the value of the .name pointer, causing a memory leak of the memory you malloc()'ed earlier.

Upvotes: 2

Related Questions