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

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

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 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?

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

* 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 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?

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

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 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?

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. +

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 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?

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. +

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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 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?

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

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

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. +

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers