Does malloc() allocate a contiguous block of memory

c++coding-stylemalloc

I have a piece of code written by a very old school programmer 🙂 . it goes something like this

typedef struct ts_request
{ 
  ts_request_buffer_header_def header; 
  char                         package[1]; 
} ts_request_def; 

ts_request_def* request_buffer = 
malloc(sizeof(ts_request_def) + (2 * 1024 * 1024));

the programmer basically is working on a buffer overflow concept. I know the code looks dodgy. so my questions are:

Does malloc always allocate contiguous block of memory? because in this code if the blocks are not contiguous, the code will fail big time
Doing free(request_buffer) , will it free all the bytes allocated by malloc i.e sizeof(ts_request_def) + (2 * 1024 * 1024),
or only the bytes of the size of the structure sizeof(ts_request_def)
Do you see any evident problems with this approach, I need to discuss this with my boss and would like to point out any loopholes with this approach

Best Solution

To answer your numbered points.

Yes.
All the bytes. Malloc/free doesn't know or care about the type of the object, just the size.
It is strictly speaking undefined behaviour, but a common trick supported by many implementations. See below for other alternatives.

The latest C standard, ISO/IEC 9899:1999 (informally C99), allows flexible array members.

An example of this would be:

int main(void)
{       
    struct { size_t x; char a[]; } *p;
    p = malloc(sizeof *p + 100);
    if (p)
    {
        /* You can now access up to p->a[99] safely */
    }
}

This now standardized feature allowed you to avoid using the common, but non-standard, implementation extension that you describe in your question. Strictly speaking, using a non-flexible array member and accessing beyond its bounds is undefined behaviour, but many implementations document and encourage it.

Furthermore, gcc allows zero-length arrays as an extension. Zero-length arrays are illegal in standard C, but gcc introduced this feature before C99 gave us flexible array members.

In a response to a comment, I will explain why the snippet below is technically undefined behaviour. Section numbers I quote refer to C99 (ISO/IEC 9899:1999)

struct {
    char arr[1];
} *x;
x = malloc(sizeof *x + 1024);
x->arr[23] = 42;

Firstly, 6.5.2.1#2 shows a[i] is identical to (*((a)+(i))), so x->arr[23] is equivalent to (*((x->arr)+(23))). Now, 6.5.6#8 (on the addition of a pointer and an integer) says:

"If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined."

For this reason, because x->arr[23] is not within the array, the behaviour is undefined. You might still think that it's okay because the malloc() implies the array has now been extended, but this is not strictly the case. Informative Annex J.2 (which lists examples of undefined behaviour) provides further clarification with an example:

An array subscript is out of range, even if an object is apparently accessible with the given subscript (as in the lvalue expression a[1][7] given the declaration int a[4][5]) (6.5.6).

Related Solutions

What does “static” mean in C

A static variable inside a function keeps its value between invocations.
A static global variable or a function is "seen" only in the file it's declared in

(1) is the more foreign topic if you're a newbie, so here's an example:

#include <stdio.h>

void foo()
{
    int a = 10;
    static int sa = 10;

    a += 5;
    sa += 5;

    printf("a = %d, sa = %d\n", a, sa);
}


int main()
{
    int i;

    for (i = 0; i < 10; ++i)
        foo();
}

This prints:

a = 15, sa = 15
a = 15, sa = 20
a = 15, sa = 25
a = 15, sa = 30
a = 15, sa = 35
a = 15, sa = 40
a = 15, sa = 45
a = 15, sa = 50
a = 15, sa = 55
a = 15, sa = 60

This is useful for cases where a function needs to keep some state between invocations, and you don't want to use global variables. Beware, however, this feature should be used very sparingly - it makes your code not thread-safe and harder to understand.

(2) Is used widely as an "access control" feature. If you have a .c file implementing some functionality, it usually exposes only a few "public" functions to users. The rest of its functions should be made static, so that the user won't be able to access them. This is encapsulation, a good practice.

Quoting Wikipedia:

In the C programming language, static is used with global variables and functions to set their scope to the containing file. In local variables, static is used to store the variable in the statically allocated memory instead of the automatically allocated memory. While the language does not dictate the implementation of either type of memory, statically allocated memory is typically reserved in data segment of the program at compile time, while the automatically allocated memory is normally implemented as a transient call stack.

And to answer your second question, it's not like in C#.

In C++, however, static is also used to define class attributes (shared between all objects of the same class) and methods. In C there are no classes, so this feature is irrelevant.

Do I cast the result of malloc

TL;DR

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

has two problems. The cast and that you're using the type instead of variable as argument for sizeof. Instead, do like this:

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

Long version

No; you don't cast the result, since:

It is unnecessary, as void * is automatically and safely promoted to any other pointer type in this case.
It adds clutter to the code, casts are not very easy to read (especially if the pointer type is long).
It makes you repeat yourself, which is generally bad.
It can hide an error if you forgot to include <stdlib.h>. This can cause crashes (or, worse, not cause a crash until way later in some totally different part of the code). Consider what happens if pointers and integers are differently sized; then you're hiding a warning by casting and might lose bits of your returned address. Note: as of C99 implicit functions are gone from C, and this point is no longer relevant since there's no automatic assumption that undeclared functions return int.

As a clarification, note that I said "you don't cast", not "you don't need to cast". In my opinion, it's a failure to include the cast, even if you got it right. There are simply no benefits to doing it, but a bunch of potential risks, and including the cast indicates that you don't know about the risks.

Also note, as commentators point out, that the above talks about straight C, not C++. I very firmly believe in C and C++ as separate languages.

To add further, your code needlessly repeats the type information (int) which can cause errors. It's better to de-reference the pointer being used to store the return value, to "lock" the two together:

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

This also moves the length to the front for increased visibility, and drops the redundant parentheses with sizeof; they are only needed when the argument is a type name. Many people seem to not know (or ignore) this, which makes their code more verbose. Remember: sizeof is not a function! :)

While moving length to the front may increase visibility in some rare cases, one should also pay attention that in the general case, it should be better to write the expression as:

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

Since keeping the sizeof first, in this case, ensures multiplication is done with at least size_t math.

Compare: malloc(sizeof *sieve * length * width) vs. malloc(length * width * sizeof *sieve) the second may overflow the length * width when width and length are smaller types than size_t.