GNOME Bugzilla – Bug 122973
Cleanups to import of glib/gnulib
Last modified: 2004-12-22 21:47:04 UTC
The code in glib/gnulib is broken for all platforms (and even Linux when --enable-included-printf is specified). The attached patch fixes all problems: 1. [configure.in] Don't define HAVE_C99_VSNPRINTF and HAVE_C99_SNPRINTF when the included printf is used 2. [glib/gnulib/README] Fix up glib/gnulib/README 3. [glib/gnulib/g-gnulib.h, glib/gnulib/printf.c] Setting HAVE_SNPRINTF depending on HAVE_C99_SNPRINTF is confusing. In glib/gnulib/printf.c use HAVE_C99_SNPRINTF instead. Accomplishes the same thing while not being confusing. 4. [glib/gnulib/printf.c] Fix off-by-one errors in glib/gnulib/printf.c. The _g_gnulib_v*printf functions seem to set length to the length of the string + 1 for the trailing '\0';
Created attachment 20195 [details] [review] Patch
I committed 1 and 2, decided that 3 isn't really confusing (the aim here is to stay as close to gnulib as possible, thus we're doing some modifications via cpp rather than sed). Regarding 4, I agree that fprintf must not write the trailing nul out to the file, but thats the only off-by-one error I see. If there are any others, testcases would be appreciated.
Everything referencing length+1 is an off-by-one error. On GNU/Linux, force building of gnulib by passing --enable-included-printf to ./configure. Then, look at the output of the files created by glib-genmarshal. You'll see \0 characters in the output. The tests pass but these characters exist in the output.
No, it is not. length refers to the length of the string without the trailing nul, so if you want to copy the string including the trailing nul, you have to use length + 1. That covers all uses besides the one in fprintf() which is reponsible for the glib-genmarshal problems you mention, and which I fixed yesterday...
_g_gnulib_vfprintf() calls vasnprintf. This contains an off-by-one error we both agree on. However, _g_gnulib_vsprintf() also calls vasnprintf. How is length+1 valid in _g_gnulib_vsprintf() and not in _g_gnulib_vfprintf()?
length is the length of the returned string, ie strlen(result). length + 1 is the number of bytes which has been allocated. length + 1 wasn't invalid in fprintf in the sense of array bounds violation, but fprintf shouldn't write the final byte (which is the trailing nul) to the file.