Thread: pgsql: to_char(): have format 'OF' only show the leading negative sign
to_char(): have format 'OF' only show the leading negative sign Previously both hours and minutes displayed as negative. Report by David Pozsar Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/9b43d73b3f9bef276a46660920a01f0421c4323a Modified Files -------------- src/backend/utils/adt/formatting.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Bruce,
* 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 argument of
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?
Thanks!
Stephen
Attachment
Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign
From
Robert Haas
Date:
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 argument oftype ‘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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign
From
Stephen Frost
Date:
* 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.
Thanks!
Stephen
Attachment
On Wed, Apr 29, 2015 at 10:11:02AM -0400, Stephen Frost wrote: > Bruce, > > * 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 argument oftype ‘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? OK, I have switched to labs() and will keep an eye on the buildfarm; patch attached. (There is already an uncomfortable amount of red there.) -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Attachment
Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign
From
Bruce Momjian
Date:
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? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign
From
Robert Haas
Date:
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
Re: Re: pgsql: to_char(): have format 'OF' only show the leading negative sign
From
Bruce Momjian
Date:
On Thu, Apr 30, 2015 at 09:07:45AM -0400, Robert Haas wrote: > >> > > 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. I think abs() is fine; there is no risk of overflow from mod; thanks. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +