Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign

On Wed, Apr 29, 2015 at 5:05 PM, Bruce Momjian <bruce@momjian.us> wrote:
> On Wed, Apr 29, 2015 at 02:22:22PM -0400, Stephen Frost wrote:
>> * Robert Haas (robertmhaas@gmail.com) wrote:
>> > On Wed, Apr 29, 2015 at 10:11 AM, Stephen Frost <sfrost@snowman.net> wrote:
>> > > * Bruce Momjian (bruce@momjian.us) wrote:
>> > >> to_char():  have format 'OF' only show the leading negative sign
>> > >>
>> > >> Previously both hours and minutes displayed as negative.
>> > >>
>> > >> Report by David Pozsar
>> > >
>> > > This is causing the following error for me:
>> > >
>> > > /home/sfrost/git/pg/dev/postgresql/src/backend/utils/adt/formatting.c: In function ‘DCH_to_char’:
>> > > /home/sfrost/git/pg/dev/postgresql/src/backend/utils/adt/formatting.c:2510:6: warning: format ‘%ld’ expects
argumentof type ‘long int’, but argument 3 has type ‘int’ [-Wformat=] 
>> > >       sprintf(s, ":%02ld", abs(tm->tm_gmtoff % SECS_PER_HOUR) / SECS_PER_MINUTE);
>> > >       ^
>> > >
>> > > Since abs() is declared to return just an int.  I don't see it anywhere
>> > > in our tree and I'm not sure how portable it actually is, but labs() is
>> > > supposedly in C89, so perhaps that should be used here instead?
>> >
>> > I just removed the "l" for now, since %02d is used to print the result
>> > of abs() elsewhere.  If that isn't right, someone can re-fix my fix.
>>
>> Works for me.  I'm no longer seeing the warning.
>
> Oh, I see now that my commit generated a conflict and didn't apply.
> Should we try labs() or just ignore it?

If the result is guaranteed to fit in an integer, then there's no
reason to adopt labs.  If you think there are platforms where it would
overflow int but fit into long, then there might be a reason to change
it.  All things being equal, it'd be nicer if we didn't need to use
labs(), because every function we depend on is another thing that can
break on some obscure platform.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company