GNOME Bugzilla – Bug 55106
Better handling of positional parameters
Last modified: 2011-02-18 15:47:08 UTC
This buffer is for notes you don't want to save, and for Lisp evaluation. If you want to create a file, visit that file with C-x C-f, then enter the text in that file's own buffer. It would be very difficult to get GLib to actually handle positional parameters, but the handling could be improved a bit from what it is now. 1) When it warns about positional parameters, it should simply skip over the entire format specifier. Right now it tries to handle the parameter as if it wasn't positional, which usually results in a crash. 2) It should also warn about the positional variant of the '*' length specifier. http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=41071
Here is a patch which adds a minimal fix to make valid formats with positional parameters work - including positional widths or precisions. It is not necessary to warn about them separately, since a) mixing positional and non-positional pieces is undefined anyway and will also cause segfaults from glibcs printf. b) the format parsing is so poor the we happen to warn anyway if we meet a $ somewhere between % and the conversion specification. Note that a) we could do a bit better on positional parameters by not just add 1k for each $ we find but rather continue to parse the stuff, just don't look at any va_args anymore, but rather assume reasonable values for the length of the conversion. (eg it seems pointless to allocate 1k for %2$c or for %1$.1s, while it may be ok for something like %3$s when we can't get at the actual string length). b) It would be good to avoid the warning for positional parameters, since I guess this mandatory warning will scare people away from positional parameters - which are very much necessary for high-quality localization.
Created attachment 5066 [details] [review] the patch
Either we have to handle positional parameters properly (think about the fact that %1$s could be more than 1024 characters long), or we should noisely complain in a way that will force people to remove the format with positional parameters. Unfortunately, handling positional parameters has two problems: a) It is very hard to do without lots of complexity. b) positional parameters are not in ANSI C and aren't in what we portably can assuem for the GTK+ stack. So, by allowing for positional parameters, we could at best enable people with supporting libc's to use them. And in most cases, if you are willing to assume positional parameters, you probably can assume vasprintf(). (Well, maybe not quite since positional parameters are Unix98, while apsprintf isn't. But close.)
Yes, I'm not exactly fond of writing the two-pass code needed to deal with positional parameters either. But your comment about assuming v(a)snprintf is a good one. The main gripe I have with the printf_string_upper_bound stuff is that it shouldn't be necessary to do this manual format parsing stuff if a C99 conforming vsnprintf is available. The following patch a) contains the minimal fix for manual parsing of positional parameters again b) moves the printf_string_upper_bound call in g_logv in the !HAVE_VSNPRINTF section b) adds AC_FUNC_VSNPRINTF_C99 to acinclude.m4 and uses it in configure.in c) conditionally turns g_printf_string_upper_bound into a simple vsnprintf wrapper. Comments ?
Created attachment 5606 [details] [review] the patch
Oh, btw, I would like to propose this patch for inclusion in 1.3.x, even if the target milestone of this defect is 1.2.11.
I think you should go ahead and commit this. I'm a little worried that positional parameters will sneak into the GTK+ translations now, but that really is a completely separate issue.