[vlc-devel] [PATCH] Enforce null pointer replacement on msg_Foo debug strings

[Rémi Denis-Courmont]

----- Message d'origine -----
> We can't fix va_list without broking portability and va_list can't be

if it affects only Solaris, then portability is not a problem, but multiple architectures might be.

> built through an intermediate function without having to build a custom
> format-string parser, which is too complex. Replacing the function by
> gnu's implementation isn't possible without adding lots of glibc's code.

> So I finally decided to fix every occurence of msg_Foo. Not every
> occurence might need it, but I'm more concerned about the ones I might
> have missed. Not fixing the direct calls to *snprintf.

This patch is quite bad IMHO. First, the macro is not safe w.r.t. parameter expansion. That's likely to cause problems down the line, i.e. if the parameter evaluation has side effects.

Second, it is totally blind. In some cases the string cannot be NULL. And that hides another problem: some developpers will not use it because it is useless in some cases. Then the code changes and the bug reappers silently. It could also be that the macro is required but the dev forgot it.

In other words, this is an invasive patch that does not really solve the problem.

There may be simpler implementations of sprintf than glibc, e.g. gnulib or mingw. Another option is to map page 0 and write "(null)\0" as the firt 7 bytes.