On Mon, Jul 29, 2024 at 10:36 AM Bruno Haible <br...@clisp.org> wrote:
>
> I see two — mostly unrelated — topics in your mail and the referenced
> threads.
>
> 1) The coding paradigm.
>
>    30 years ago I saw a piece of code from Microsoft, which called
>    fopen() and assumed that the return value was != NULL.
>
>    Nowadays, "Solar Designer" presents us code like this:
>      char *text;
>      vasprintf (&text, format, args);
>    that ignores the return value of vasprintf, observes that it is
>    unsafe, and claims that the behaviour of vasprintf should be
>    changed.
>
>    This is not convincing me. Proper coding with C APIs implies to
>    *check the return values*.
>
>    The only reasonable change that should be done is to mark the
>    declaration of asprintf and vasprintf with attribute
>    __warn_unused_result__. This way, the compiler will warn about
>    the crappy code.
>
> 2) The behaviour of asprintf() and vasprintf().
>
>    What can happen with the result pointer ptr if
>      asprintf (&ptr, ...)
>    fails? There are four possibilities:
>
>      (a) ptr is unchanged.
>      (b) ptr is set to NULL.
>      (c) ptr is set to a non-NULL pointer that is not free()able.
>      (d) ptr is set to a non-NULL pointer that is freshly allocated
>          and thus meant to be freed.
>
>    It is quite obvious that no reasonable implementation will do (c) nor (d).
>    (c) because that would be an invitation for doing free(invalid_pointer).
>    (d) because what if the function cannot do this fresh allocation, since
>    memory is tight?
>
>    The manual page referenced by
>    <https://bugs.freedesktop.org/show_bug.cgi?id=90017>
>    allows all of (a), (b), (c), (d), and shows a case where code would
>    be buggy if an implementation would do (c).
>
>    POSIX
>    <https://pubs.opengroup.org/onlinepubs/9799919799/functions/asprintf.html>
>    quote: "For asprintf(), if memory allocation was not possible, or if
>            some other error occurs, the function shall return a negative
>            value, and the contents of the location referenced by ptr are
>            undefined, but shall not refer to allocated memory."
>    allows (a), (b), (c), but not (d).

Forgive my ignorance... it seems (b) is the only option that satisfies
" but shall not refer to allocated memory" since a stack allocation is
still an allocation.

>    So, I hope everyone agrees that neither glibc nor gnulib should do
>    (c) or (d).
>
>    The traditional behaviour is (a).
>
>    I have no strong objections regarding (b). Assigning NULL is just
>    unnecessary: A programmer who wants ptr == NULL in case of failure
>    can just assign 'ptr = NULL;' before the call.
>
>    Florian Weimer can add extra '*resultp = NULL;' assignments in glibc,
>    if he likes to, for the sake of crappy user code that does not check
>    return values. But I don't think this needs to become commonplace,
>    and in particular I don't think Gnulib needs that.

You should consider making the interface easy to use correctly, and
hard to use incorrectly. That will help novice programmers avoid
mistakes. In that light, set the ptr to NULL to make it as safe as
possible.

Jeff

Reply via email to