On Thursday 18 June 2009 22:39, Colin Watson wrote:
> 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.
Well, it may change, but it never goes to 0:
"No function in this volume of IEEE Std 1003.1-2001 shall set errno to 0."
And that's the only thing we care about: errno != 0 -> bad.
> > 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.
So you are saying that successful printf may set errno to nonzero?!
POSIX manpage says:
ERRORS
For the conditions under which fprintf() and printf() fail and may
fail, refer to fputc() or fputwc() .
In addition, all forms of fprintf() may fail if:
EILSEQ A wide-character code that does not correspond to a valid char-
acter has been detected.
EINVAL There are insufficient arguments.
The printf() and fprintf() functions may fail if:
ENOMEM Insufficient storage space is available.
Where do you think they mean they store EINVAL etc?
Obviously, errno.
There is a reason why I want to simplify the code:
Your patch:
# make bloatcheck
function old new delta
print_direc 427 481 +54
printf_main 834 842 +8
multiconvert 79 82 +3
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 65/0) Total: 65 bytes
text data bss dec hex filename
820667 458 6956 828081 ca2b1 busybox_old
820732 458 6956 828146 ca2f2 busybox_unstripped
My adaptation of your patch:
# make bloatcheck
function old new delta
printf_main 834 827 -7
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-7) Total: -7 bytes
text data bss dec hex filename
820667 458 6956 828081 ca2b1 busybox_old
820660 458 6956 828074 ca2aa busybox_unstripped
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
