0
char (*cHighValue)[20];

cHighValue = malloc (X * sizeof(char *));

for (i = 0; i < X; ++i)
{
    cHighValue [i] = (char *) malloc (20 * sizeof(char));
}

gives me error : Expression must be a modifiable lvalue, for "cHighValue [i] = (char *) malloc (20 * sizeof(char));" Why?

A.J
  • 725
  • 10
  • 33
  • 2
    `cHighValue[i]` is a `char`, not a pointer. – Oliver Charlesworth Mar 31 '14 at 07:38
  • Actually a possible duplicate of [How do I correctly set up, access, and free a multidimensional array in C?](http://stackoverflow.com/questions/12462615/how-do-i-correctly-set-up-access-and-free-a-multidimensional-array-in-c) – Lundin Mar 31 '14 at 08:11

5 Answers5

3

cHighValue is a pointer to a char array.

Allocate as

cHighValue=malloc(sizeof(char)*20*X);
HelloWorld123456789
  • 5,299
  • 3
  • 23
  • 33
  • What's the point of writing `sizeof(char)`? It is 1 by definition. – glglgl Mar 31 '14 at 07:39
  • It helps to understand that it is indeed `char` and not `char *`. – HelloWorld123456789 Mar 31 '14 at 07:42
  • Then I would write `malloc(sizeof(*cHighValue)*20*X)`. – glglgl Mar 31 '14 at 07:44
  • 1
    @glglgl There is nothing wrong with `sizeof(char)`. It is readable and therefore prevented him from writing a bug. You however, wrote a fatal bug because you wrote `sizeof(*cHighValue)`. cHighValue is an array pointer, so you allocate far too much memory. Keep things simple instead. – Lundin Mar 31 '14 at 07:55
  • @Lundin There you are right, I didn't look twice. But again: writing `sizeof(char)` is pointless in my eyes, as it is always `1`. – glglgl Mar 31 '14 at 08:20
  • @Lundin And, your understanding of "fatal" is not common sense. "fatal" would have been allocating *too few* memory. And, who tells me that the user doesn't want to allocate space for `X` 20 byte arrays? – glglgl Mar 31 '14 at 08:22
1

You are declaring cHighValue as a pointer to an array of 20 chars. However in your code you use it as being a pointer to array of pointers. What you probably want is to declare cHighValue as an array of pointers and because you allocate it on heap you have to declare it as a pointer to pointer. I.e.:

char **cHighValue;
Marian
  • 7,402
  • 2
  • 22
  • 34
  • Hey! That's not fair. :) – HelloWorld123456789 Mar 31 '14 at 07:48
  • @RikayanBandyopadhyay frankly it is only now I see you have posted the second part of my answer before. I've up-voted your answer as recompense ... – Marian Mar 31 '14 at 07:51
  • -1 for "You have to declare it as a pointer to pointer". That's not true. Allocate a 2D array as a 2D array, not as something else... – Lundin Mar 31 '14 at 08:08
  • @Lundin I do not understand what you mind. I've answered the question why the code does not work. The asker clearly allocates array of pointers and then fills all pointers. To make this code to work, the type of the variable in question shall be pointer to pointer. – Marian Mar 31 '14 at 11:14
  • @Marian The original question seems to be about allocating a 2D array. In that case, he should leave the array pointer as it is and fix the malloc instead. An array of pointers is not a 2D array, it just happens to have the same indexing syntax. – Lundin Mar 31 '14 at 11:22
0

cHighValue is a pointer to 20-char array, so cHighValue[i] is an i-th 20-byte-long array of chars.
And the array of chars is not a modifiable lvalue, which could be assigned a pointer value returned by malloc().

To achieve what you (probably) want, remove parentheses from the cHighValue declaration.

CiaPan
  • 9,381
  • 2
  • 21
  • 35
0

The proper way to allocate a two-dimensional array would be:

char (*cHighValue)[Y];

cHighValue = malloc( sizeof(char[X][Y]) );

In particular, you should note:

  • Do not use multiple mallocs, because they will not give you a true 2D array, but instead a fragmented pointer-to-pointer lookup table which is allocated all over the heap. Such a lookup table cannot be used with fundamental functions like memset, memcpy etc.
  • Casting the result of malloc is pointless in C.
Lundin
  • 195,001
  • 40
  • 254
  • 396
-1

It's the type of the array,

char* cHighValue[20];


cHighValue[0] = malloc (X * sizeof(char *));

for (i = 0; i < X; ++i)
{
     cHighValue[i] = (char *) malloc (20 * sizeof(char));
}
angeek86
  • 108
  • 5