malloc() can return NULL on json_array_new(0)

[sasdallas]

Hi!

I'm a bit confused on your documentation in the README. I'm incorporating this library into my OS project, and I was testing it via your example code.

Here's the code that I'm using:

json_value * arr = json_array_new(0);

dprintf(DEBUG, "array->type = %i (array = 0x%x)\n", arr->type, arr);
json_array_push(arr, json_string_new("Hello world!"));
json_array_push(arr, json_integer_new(128));

char * buf = malloc(json_measure(arr));
json_serialize(buf, arr);

dprintf(DEBUG, "%s\n", buf);

When this code is run, the assertion fails because the array is allocated to 0x0. I traced that to this section of code:

json_value * json_array_new (size_t length)
{
    json_value * value = (json_value *) calloc (1, sizeof (json_builder_value));

    if (!value)
       return NULL;

    ((json_builder_value *) value)->is_builder_value = 1;

    value->type = json_array;

    if (! (value->u.array.values = (json_value **) malloc (length * sizeof (json_value *))))
    {
       free (value);
       return NULL;
    }

    ((json_builder_value *) value)->additional_length_allocated = length;

    return value;
}

This section is problematic:

    if (! (value->u.array.values = (json_value **) malloc (length * sizeof (json_value *))))
    {
       free (value);
       return NULL;
    }

If length is 0 (which it is), then this equivocates to malloc(0), which makes no sense - according to this page, if size is 0, then it may or may not return a NULL pointer, it is impl. specific (and that this pointer must not be dereferenced).

Is this intentional behavior, or am I missing something?

[LB--]

Good catch, that does look like a bug to me. Do you think adding && length > 0 as a second condition to that if statement would let it work fine in both cases? I'll have to dig into this more later, but generally the idea is that the result of malloc(0) is often later passed to realloc which is safe in both cases, and free is also safe in both cases.

[sasdallas]

Good catch, that does look like a bug to me. Do you think adding && length > 0 as a second condition to that if statement would let it work fine in both cases? I'll have to dig into this more later, but generally the idea is that the result of malloc(0) is often later passed to realloc which is safe in both cases, and free is also safe in both cases.

Yes - adding that clause should fix the problem, I believe. free is safe in both cases and by C standard realloc should work fine.