0

My professor told me that the following code is incorrect:

int *array = malloc(sizeof *array * length);

And that it should instead be:

int *array = malloc(length * sizeof(int));

He said that you're supposed to encase the type between brackets for sizeof, and that length must come before the sizeof operator. He said that I can use *array instead of int, but that he preferred the latter.

My biggest question is why is it that length must come before size when calling malloc, but also why it's preferrable to use the pointer type instead of the pointer itself with sizeof.

He also mentioned that casting (i.e. (int*)) is desirable, but I'm not sure why it is necessary.

  • 4
    Amazing how often teachers conflate their personal preferences with what's "correct". The first form is more robust. Your prof also is wrong on the casting bit. Seems they are using a C++ compiler to compile C code, and are unaware of the subtleties in your original **correct** code. – StoryTeller - Unslander Monica Mar 16 '22 at 19:59
  • 2
    Have a read [here](https://stackoverflow.com/q/605845/817643). You can try showing it to your profs, but I doubt the commutative experience of thousands of developers will sway someone in academia whose already made up their mind. You may have to do what your prof says to gain points, but don't forget what you learn here for when you are no longer under the professor's shackles. – StoryTeller - Unslander Monica Mar 16 '22 at 20:03
  • 1
    See also [this one](https://stackoverflow.com/q/13120473/10871073) about when you *need* to use brackets around the operand of `sizeof`. – Adrian Mole Mar 16 '22 at 20:04

4 Answers4

3

Both are valid, but many veteran programmers will prefer the way you did it.

The advantage of using sizeof *array as opposed to sizeof(int) is that if you happen to change the type of array then you don't need to change how you do the allocation.

There's also no technical reason to multiply by length first instead of the element size. If anything, when looking at a call to malloc the first thing you want to know is how many "things" you're allocating, so from a readability standpoint putting the length first might make more sense. On the other hand, because the result of the sizeof operator is unsigned, putting it first guarantees that the math is done with unsigned types if you have multiple array dimensions.

You also don't want to cast the return value of malloc as that can mask other errors in your code, specifically a missing #include <stdlib.h>

dbush
  • 205,898
  • 23
  • 218
  • 273
  • 1
    I recall it being mentioned somewhere that for a multi-dimensional array being allocated like this, it's best to do `sizeof *array * row_len * col_len` - that way the math is always done in unsigned integers (size_t), and overflow is less of a concern. – StoryTeller - Unslander Monica Mar 16 '22 at 20:05
  • @StoryTeller-UnslanderMonica Ah, good point! – dbush Mar 16 '22 at 20:08
1

The sizeof operator is defined the following way

sizeof unary-expression

So this line

int *array = malloc(sizeof *array * length);

is equivalent to

int *array = malloc( ( sizeof *array ) * length);

But the original line though it is correct is less clear. So the preferable way to write this line is

int *array = malloc( length * sizeof *array );

or even better

int *array = malloc( length * sizeof( *array ) );

You may write also like

int *array = malloc(length * sizeof(int));

but if the type of the elements of the dynamically allocated array will be changed then you need to make changes in two places like for example

long *array = malloc(length * sizeof(long));
^^^^                                 ^^^^

If you will forget to change the type in the sizeof expression then there can be a bug that will be difficult to find in a big project.

In this line

int *array = malloc( length * sizeof( *array ) );

you need make a change only in one place

long *array = malloc( length * sizeof( *array ) );
^^^^

So this declaration is less error prone.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

The only problem with the first one is this (I changed from 'array' to a name that doesnt carry such obvious connotations)

int *twoodle = malloc(sizeof *twoodle * length);

int *twoodle = malloc(sizeof *twoodle);

Both are valid, will compile, but only one is correct. And its impossible to tell just by looking at it

I know that the other format has the same problem but people think (it was stated above ) "So this declaration is less error prone.". But you can still make errors here too

Point is , make sure your mallocs are correct (duh)

pm100
  • 48,078
  • 23
  • 82
  • 145
1

int *array = malloc(length * sizeof(int)); poses another problem when length is the product of int multiplication.

Consider

  int *alloc_array(int rows, int col) {
    int *array = malloc(rows * col * sizeof *array);  // Overflow more likely
    return array;
  }

versus

  int *array = malloc(sizeof *array * rows * col); // Better

In the first, rows * col is int math and overflows (UB) if the product exceeds INT_MAX.

In the 2nd, multiplication is done using size_t math. Overflow occurs when product exceeds SIZE_MAX.

SIZE_MAX is often twice are great as INT_MAX Sometimes 8,000,000,000 times larger than INT_MAX.

Lead your multiplication with the wider type.


// Use this in the stuck in the 90s prof's class.
int *array = (int *) malloc(length * sizeof(int));

// Otherwise, use
int *array = malloc(sizeof *array * length);

Realize that with C, from time-to-time, you will be obliged to code to some arcane standards. (IMO, often not for technical reasons.) All languages have this issue. C simply has a longer history.


Just noticed @dbush already covered some of this this.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256