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

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

In response to

Responses

Browse pgsql-hackers by date

  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