From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | pavel(dot)stehule(at)gmail(dot)com |
Cc: | dean(dot)a(dot)rasheed(at)gmail(dot)com, sfrost(at)snowman(dot)net, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Re: proposal: a width specification for s specifier (format function), fix behave when positional and ordered placeholders are used |
Date: | 2013-03-05 10:23:26 |
Message-ID: | 20130305.192326.79813028.horiguchi.kyotaro@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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,
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.
Finally, enlargeStringInfo fails just after for large numbers. We
might should keep it under certain length to get rid of memory
exhaustion.
Anyway, I'll send this patch to committers as it is in this
message.
best wishes,
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachment | Content-Type | Size |
---|---|---|
format-width-20130305.patch | text/x-patch | 25.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2013-03-05 10:53:10 | Re: 9.2.3 crashes during archive recovery |
Previous Message | Heikki Linnakangas | 2013-03-05 09:35:14 | Re: Enabling Checksums |