2

I have fullNames, which is a 2D array that has sorted full names in it and I want to copy its content into sortedNames, which is a 2D array that exists out side of this function. (I get ***sortedNames as a parameter).

I dynamically allocated this array, but the copying does not succeed. The program crashes after the 4th attempt to copy a name from fullNames to sortedNames. Why?

stringcpy and stringlen are functions that I created. They do the same thing as strcpy and strlen does.

/*allocating memory for sortedNames*/
*sortedNames = (char**) malloc(n);/*n is the number of names*/

/*allocating memory for each sortedNames array*/
for (i = 0; i < n; i++)
{
    (*sortedNames)[i] = (char*) malloc(stringlen(fullNames[i])+1);
}


/*Copying fullNames into sortedNames*/

for (i = 0; i < n; i++)
{
    stringcpy((*sortedNames)[i],fullNames[i]);
}
Dimitri Dewaele
  • 10,311
  • 21
  • 80
  • 127
itamar8910
  • 195
  • 3
  • 7
  • Is n the number of names or the total size to be allocated> – Sourav Kanta Jun 03 '15 at 07:31
  • 2
    [Obligatory reminder about not casting the reuslt of malloc in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Paul R Jun 03 '15 at 07:33
  • You should modify your question to include the function prototype. It is `void myfunction(char ***sortedNames, const char **fullNames, size_t n)` ? – chqrlie Jun 03 '15 at 07:34

2 Answers2

4

You do not allocate enough memory for the array of pointers, you should allocate this way:

*sortedNames = (char**)malloc(n * sizeof(char *));

Furthermore, why not use strlen and strcpy in place of stringlen and stringcpy? It this just a typo or do these function perform some extra function?

Regarding the cast on malloc return value, you could remove it if you do not intend to compile your code as C++ and write this:

*sortedNames = malloc(n * sizeof(**sortedNames));

Regarding the extra parentheses around **sortedNames, be aware that they are not necessary so you can remove them or not depending on your local style conventions.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 2
    [Obligatory reminder about not casting the reuslt of malloc in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Paul R Jun 03 '15 at 07:33
  • Or simply `*sortedNames = malloc (n * sizeof **sortedNames);` – David C. Rankin Jun 03 '15 at 07:37
  • @Paul R: I beg to disagree, especially given the lack of information in the question. Nowhere do we know the type of `sortedNames`. The explicit cast will prevent compilation if the type is not indeed `char ***sortedNames` or equivalent. – chqrlie Jun 03 '15 at 07:42
  • Well even in the particular case you mention you're just trading one potential problem for another. Casts in general (and especially redundant casts) are a code smell, and should be avoided other than as a last resort when no other more appropriate solution is available – Paul R Jun 03 '15 at 07:46
  • @Paul R: I agree with you about casts in general. In this particular case, I would use a macro to enforce type checking: `#define ALLOC(type, n) ((type*)calloc(n, sizeof(type)))`. – chqrlie Jun 03 '15 at 07:50
  • Thank you, works perfectly. Regarding the functions stringlen & stringcopy: It was a part of a homework assignment, and it was instructed not to use functions from stlib.h, and rather write them on my own. – itamar8910 Jun 12 '15 at 13:18
0

There should be 2 edits as the memory allocated may not be sufficient. This code :

(*sortedNames)[i] = (char*) malloc(n);

allocates memory for n bytes whiles you need memory for (n*the size of string) bytes.The second malloc may work as char occupies 1 byte. But try to use sizeof() to make it system independent.

The correct code would be :

 (*sortedNames)[i] = malloc(n*sizeof(char *));
Sourav Kanta
  • 2,727
  • 1
  • 18
  • 29
  • 1
    Need the same comment by @PaulR :-) – Sourav Ghosh Jun 03 '15 at 07:38
  • 1
    Why, oh why, do you **cast malloc** -> don't. Meaning **get rid of** `(char *)`. – David C. Rankin Jun 03 '15 at 07:39
  • I beg to disagree, especially given the lack of information in the question. Nowhere do we know the type of `sortedNames`. The explicit cast will prevent compilation if the type is not indeed `char ***sortedNames` or equivalent. – chqrlie Jun 03 '15 at 07:41