←back to thread

237 points ekr____ | 1 comments | | HN request time: 0.212s | source
Show context
bluetomcat ◴[] No.42724685[source]
This isn't proper usage of realloc:

    lines = realloc(lines, (num_lines + 1) * sizeof(char *));
In case it cannot service the reallocation and returns NULL, it will overwrite "lines" with NULL, but the memory that "lines" referred to is still there and needs to be either freed or used.

The proper way to call it would be:

    tmp = realloc(lines, (num_lines + 1) * sizeof(char *));

    if (tmp == NULL) {
        free(lines);
        lines = NULL;
        // ... possibly exit the program (without a memory leak)
    } else {
        lines = tmp;
    }
replies(9): >>42724759 #>>42724866 #>>42725435 #>>42726629 #>>42727024 #>>42728450 #>>42728785 #>>42729894 #>>42734023 #
aa-jv ◴[] No.42724866[source]
Actually, no. You've just committed one of the cardinal sins of the *alloc()'s, which is: NULL is an acceptable return, so errno != 0 is the only way to tell if things have gone awry.

The proper use of realloc is to check errno always ... because in fact it can return NULL in a case which is not considered an error: lines is not NULL but requested size is zero. This is not considered an error case.

So, in your fix, please replace all checking of tmp == NULL, instead with checking errno != 0. Only then will you have actually fixed the OP's unsafe, incorrect code.

replies(2): >>42724929 #>>42724961 #
1. cozzyd ◴[] No.42724929[source]
In this case if (num_lines+1)(sizeof (char)) is zero that is certainly unintended