Re: Efficient output for integer types

On Mon, Sep 23, 2019 at 01:16:36PM +0100, Andrew Gierth wrote:
> >>>>> "David" == David Fetter <david@fetter.org> writes:
> 
>  David> + return pg_ltostr_zeropad(str, (uint32)0 - (uint32)value, minwidth - 1);
> 
> No, this is just reintroducing the undefined behavior again. Once the
> value has been converted to unsigned you can't cast it back to signed or
> pass it to a function expecting a signed value, since it will overflow
> in the INT_MIN case. (and in this example would probably output '-'
> signs until it ran off the end of memory).
> 
> Here's how I would do it:
> 
> char *
> pg_ltostr_zeropad(char *str, int32 value, int32 minwidth)
> {
>     int32        len;
>     uint32        uvalue = value;
> 
>     Assert(minwidth > 0);
> 
>     if (value >= 0)
>     {
>         if (value < 100 && minwidth == 2) /* Short cut for common case */
>         {
>             memcpy(str, DIGIT_TABLE + value*2, 2);
>             return str + 2;
>         }
>     }
>     else
>     {
>         *str++ = '-';
>         minwidth -= 1;
>         uvalue = (uint32)0 - uvalue;
>     }
>             
>     len = pg_ultoa_n(uvalue, str);
>     if (len >= minwidth)
>         return str + len;
> 
>     memmove(str + minwidth - len, str, len);
>     memset(str, '0', minwidth - len);
>     return str + minwidth;
> }

Done pretty much that way.

>  David>  pg_ltostr(char *str, int32 value)
>  David> +    int32    len = pg_ultoa_n(value, str);
>  David> +    return str + len;
> 
> This seems to have lost its handling of negative numbers entirely

Given the comment that precedes it and all the use cases in the code,
I changed the signature to take an unsigned integer instead. It's
pretty clear that the intent was to add digits and only digits to the
passed-in string.

> (which doesn't say much for the regression test coverage)

I didn't see any obvious way to test functions not surfaced to SQL.
Should we have one?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate