On Mon, Jul 29, 2024 at 04:35:57PM GMT, Bruno Haible wrote: > Hi Eric, > > I see two — mostly unrelated — topics in your mail and the referenced > threads. > > 1) The coding paradigm.
> > 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. Makes sense; I would be in favor of that change in gnulib (and maybe can convince Florian to consider that in glibc) > > 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. another one: (e) a combination of (a) and (b), where SOME error paths leave ptr unchanged, but others change ptr to NULL. > > 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). > > So, I hope everyone agrees that neither glibc nor gnulib should do > (c) or (d). > > The traditional behaviour is (a). The traditional glibc behvioar is (a), although I'm not 100% certain that it was reliably (a), or if it ended up actually being (e) (that is, common error paths leave ptr unchanged but there might be some corner-case error paths that end up losing the original ptr and therefore fall back to the only other sane option of (b)). But the traditional BSD behavior is (b). > > 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. Here, you've convinced me that a __warn_unused_result__ coupled with a compiler that warns about the use of potentially uninitialized variables (and which treats asprintf() as if it leaves ptr unchanged on failure) is probably sufficient to avoid bugs even if we do not follow any glibc decision on whether to start enforcing behavior (b). On the other hand, if glibc DOES decide to do (b) to match BSD, then why would gnulib NOT want to guarantee the same portably across all platforms? -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
