Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used

2013/3/5 Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>:
> Hello,
>
>> > I think that the only other behavioural glitch I spotted was that it
>> > fails to catch one overflow case, which should generate an "out of
>> > ranger" error:
>> >
>> > select format('|%*s|', -2147483648, 'foo');
>> > Result: |foo|
>> >
>> > because -(-2147483648) = 0 in signed 32-bit integers.
>
> Ouch. Thanks for pointing out.
>
>> fixed - next other overflow check added
>
> Your change shown below seems assuming that the two's complement
> of the most negative number in integer types is identical to
> itself, and looks working as expected at least on
> linux/x86_64. But C standard defines it as undefined, (As far as
> I hear :-).
>
> | if (width != 0)
> | {
> | int32 _width = -width;
> |
> | if (SAMESIGN(width, _width))
> | ereport(ERROR,
>

this pattern was used elsewhere in pg

> Instead, I think we can deny it by simply comparing with
> INT_MIN. I modified the patch like so and put some modifications
> on styling.

ook - I have not enough expirience with this topic and I cannot say
what is preferred.

>
> Finally, enlargeStringInfo fails just after for large numbers. We
> might should keep it under certain length to get rid of memory
> exhaustion.

I though about it, but I don't know a correct value - probably any
width specification higher 1MB will be bogus and can be blocked ?? Our
VARLENA max size is 1GB so with should not be higher than 1GB ever.

what do you thinking about these limits?

Regards

Pavel

>
> Anyway, I'll send this patch to committers as it is in this
> message.
>
> best wishes,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center