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