[PATCH] printf: fix exit code on conversion failure

[Colin Watson]

On Thu, Jun 18, 2009 at 10:21:37PM +0200, Denys Vlasenko wrote:
> On Thursday 18 June 2009 17:52, Colin Watson wrote:
> > @@ -69,7 +67,9 @@ static int multiconvert(const char *arg, void *result, converter convert)
> >  	errno = 0;
> >  	convert(arg, result);
> >  	if (errno) {
> > +		int save_errno = errno;
> >  		bb_error_msg("%s: invalid number", arg);
> > +		errno = save_errno;
> >  		return 1;
> >  	}
> 
> Why?

Using stdio, syslog, etc. is not guaranteed to preserve errno. If you
want to keep it reliably, you have to save it. Relying on its value
across calls to those functions is courting unspecified behaviour.

http://www.opengroup.org/onlinepubs/009695399/functions/errno.html
states that "The setting of errno after a successful call to a function
is unspecified unless the description of that function specifies that
errno shall not be modified", and e.g. neither fflush nor syslog
specifies this.

In any case, regardless of standards-lawyering, I think most C
programmers do not expect errno to be a long-lived value and so relying
on it like that would be most unclear.

> >  static void print_direc(char *format, unsigned fmt_length,
> >  		int field_width, int precision,
> > -		const char *argument)
> > +		const char *argument, int *status)
> 
> We can just observe errno != 0 on return, no need to have int *status.

I felt that having to save and restore errno all over the place would be
more code, and much less clearly correct, than just having a separate
explicit parameter that handles the exit status.

> (BTW, if it would be needed, it's better to return it as a return value,
> not a pointer param...)

Yes, true.

> Generally, good idea.
> 
> I propose this modification.

My change to multiconvert should be kept for the reasons given above.

My changes to print_direc should be kept (with the pointer parameter ->
return value change you suggested) because print_direc calls printf all
over the place and printf is not guaranteed to preserve errno on
success.

Thanks,

-- 
Colin Watson                                       [cjwatson at ubuntu.com]