Subject: Re: lib/35401
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 01/15/2007 23:30:03
The following reply was made to PR lib/35401; it has been noted by GNATS.

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org, netbsd-bugs@NetBSD.org
Cc: 
Subject: Re: lib/35401
Date: Tue, 16 Jan 2007 00:35:30 +0100

 David Laight wrote:
 >  On Mon, Jan 15, 2007 at 02:20:02PM +0000, Christian Biere wrote:
 >  >  I suspect you're referring to add_digit() when you say "a lot". This
 >  >  variant should be a bit faster:
 >  
 >  I was thinking of the cost of the call, not the contents of it!
 >  Not to mention all the other tests you are adding.
 >  Looking more closely the function itself also doesn't work.
 >  The 'a + b < a' test doesn't work for multiplication.
 
 Yes, I'm sorry about that. Shame on me. I used a wrong threshold when testing
 it. I've corrected it and tested it for all value in [0..INT_MAX] and d in
 [0..9] this time.
 
 static inline int
 add_digit(unsigned value, unsigned int d)
 {
 	unsigned long ret;
 
 	ret = value * 10;
 	if (__predict_false(value > (INT_MAX - 9) / 10)) {
 #if INT_MAX > (ULONG_MAX - 9) / 10
 		if (ret / 10 != value)
 			return -1;
 #endif
 		if (ret > INT_MAX - d)
 			return -1;
 	}
 	return ret + d;
 }
 
 >  Also you only need to for 'stupid' values, you don't need convertions
 >  that generate INT_MAX (etc).  Any numeric conversion > (say) 100kB is
 >  clearly nonsense - whether it came from the format string itself or a "%*".
 >  Overlong string convertions might need more care, but you still don't need
 >  'exact' tests.
 
 I don't agree about this threshold but you could say the "nonsense" starts
 above (INT_MAX - 9 / 10). So except for the value, we seem to agree that there
 should be a check for some threshold. The code inside the check is never reached
 below the threshold, so the expensive checks are only done for cases in which
 they are completely neglible with respect to the rest of work. Further, for
 "nonsense" the performance is actually better now because we bail out as soon
 as possible.
   
 >  Even failing the convertion could be troublesome with code that (for
 >  example) expects asprintf() never to return NULL, or snprintf() to return
 >  a +ve number.
 
 That isn't our problem and buggy code anyway. We only have to adhere to the
 specification of this library function.
 
 >  In any case, programs that allow 'broken' format strings to be supplied
 >  are MUCH more likely to be compromised by "%n", so checking anything other
 >  that "%*" is totally pointless.
 
 >  Oh - and we don't do 'if ('0' == ch)' it is too ugly...
 
 I don't see this rule in the style file and it's not ugly either. I don't mind
 about those two lines but it's faster that way at least for certain cases, so I
 thought you'd like it.
 
 -- 
 Christian