elements in array of char* getting corrupted when copied into a new array

Question

I have come across this before, and the answer has been either to null-terminate the string, or to be sure to allocate enough memory for the string. Here is the relevant snippet of code:

for (z; z<mountable_volumes; z++)
{
main_items[z] = malloc(strlen(volumes[z])+2);
main_items[z] = volumes[z];
printf("main_items[%i]: %s\n", z, volumes[z]);
}
main_items[z] = NULL;
return main_items;

Volumes[] is correct, but when its contents get created into main_items[], it goes bad. I have tried playing with malloc, even outright allocating way more ram than necessary. I have also tried tacking on a '\0' to the end of each main_items[] element. I have tried using strcpy, strncpy, sprintf with the same results.

Here is the log from my program:

volumes[0]: Unmount /sdcard
volumes[1]: Mount /system
volumes[2]: Unmount /cache
volumes[3]: Mount /data
volumes[0]: Unmount /sdc)☻
main_items[1]: Mount /syste)☻
main_items[2]: Unmount /cac‼
main_items[3]: Mount /data

What am I missing? Thanks! I can paste more of the function if its needed.

EDIT:

here is the entire function: (I have applied the strndup() and free() tips)

char** get_mount_menu_options()
{
  Volume * device_volumes = get_device_volumes();
  num_volumes = get_num_volumes();

  char** volumes = malloc (num_volumes * sizeof (char *));

  int mountable_volumes = 0;
  int usb_storage_enabled = is_usb_storage_enabled();

  int i;
  for (i=0; i<num_volumes; i++)
  {
    volumes[i] = "";
    Volume *v = &device_volumes[i];
    char* operation;
if (is_path_mountable(v->mount_point) != -1)
      {   
    if (is_path_mounted(v->mount_point)) operation = "Unmount";
    else operation = "Mount";
    volumes[mountable_volumes] = malloc(sizeof(char*));
    printf("volumes[%i]: %s %s\n", mountable_volumes, operation, v->mount_point);
    sprintf(volumes[mountable_volumes], "%s %s", operation, v->mount_point);
    mountable_volumes++;
  }
}

char **main_items = malloc (num_volumes * sizeof (char *));

int z;
for (z=0; z<mountable_volumes; z++)
{     
main_items[z] = strndup(volumes[z], strlen(volumes[z]));
free(volumes[z]);
printf("main_items[%i]: %s\n", z, volumes[z]);
}
main_items[z] = NULL;
return main_items;
}

CURRENT LOG:

volumes[0]: Unmount /sdcard
volumes[1]: Mount /system
volumes[2]: Unmount /cache
volumes[3]: Mount /data
main_items[0]: Unmount /sdc)☻
main_items[1]: Mount /syste)☻
main_items[2]: Unmount ¿♠
main_items[3]:

Thanks everyone!

What is the type of main_items? main_items[z] = volumes[z]; seems incorrect, you should be using strncpy. Arrays cannot be assigned. — Alok Save
– Alok Save, Commented Dec 22, 2011 at 17:25
It would be helpful to at least see the declarations of main_items and volumes. — DRH
– DRH, Commented Dec 22, 2011 at 17:27
entire function has been posted. main_items is a char** declared elsewhere in the program being created by get_mount_menu_options() (the function I posted): char** main_items = get_mount_menu_options(); — raidzero
– raidzero, Commented Dec 22, 2011 at 17:51

Sangeeth Saravanaraj · Accepted Answer · 2011-12-22 17:28:40Z

2

For copying strings, you need to use the strcpy(). Assignments won't work!

strcpy(main_items[z], volumes[z]);

And moreover,

main_items[z] = malloc(strlen(volumes[z])+2);

should be

main_items[z] = (char *) malloc(strlen(volumes[z]) + 1);

And I assume main_items[z] and volumes[z] is a char *

answered Dec 22, 2011 at 17:28

Sangeeth Saravanaraj

16.8k21 gold badges72 silver badges98 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Dan Fego · Accepted Answer · 2011-12-22 17:38:16Z

2

As an alternative to everyone's examples of strcpy(), you can just use strdup(), and skip the malloc():

main_items[z] = strdup(volumes[z]);

Obviously you still have to free() your memory! This also relies on volumes[z] being NULL-terminated.

Edit: Or, as Peter Downs points out in the comments, you can use strndup() rather than relying on NULL-termination, if you know the lengths of your strings.

edited Dec 22, 2011 at 17:38

answered Dec 22, 2011 at 17:32

Dan Fego

14.1k6 gold badges52 silver badges60 bronze badges

2 Comments

Peter Downs Over a year ago

Or maybe strndup if you know the lengths.

raidzero Over a year ago

I can get the length of my strings, I will try strndup thanks.

Sergey Kalinichenko · Accepted Answer · 2011-12-22 17:25:22Z

1

main_items[z] = volumes[z]; should be strcpy(main_items[z], volumes[z]);, otherwise the memory that you have allocated on the line just above is leaked, and the pointer main_items[z] becomes aliased to the pointer in volumes[z].

answered Dec 22, 2011 at 17:25

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

5 Comments

raidzero Over a year ago

I edited the original question to include the strndup and free tips. I also added the entire function, its not very long anyway :)

Sergey Kalinichenko Over a year ago

@raidzero You could have an issue in the sprintf(volumes[mountable_volumes], "%s %s", operation, v->mount_point);: Are you sure v->mount_point is null-terminated?

raidzero Over a year ago

I assumed it is since it displays correctly when printing it, but I will try adding a null terminator to the end of it since the problem is obviously not in the bottom for loop :)

raidzero Over a year ago

I added a null terminator to v->mount_point: strcat(v->mount_point, "\0"); but the result is the same

Sergey Kalinichenko Over a year ago

@raidzero You are calling volumes[mountable_volumes] = malloc(sizeof(char*)); which is incorrect: it allocates enough space to store a pointer to char, not to store an entire string! It should be strlen(operation)+strlen(v->mount_point)+2 instead of sizeof(char*).

Collectives™ on Stack Overflow

elements in array of char* getting corrupted when copied into a new array

3 Answers 3

Comments

2 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related