On Wed, Apr 13, 2016 at 9:38 AM, Tom Lane <[email protected]> wrote: > Robert Haas <[email protected]> writes: >> On Wed, Apr 13, 2016 at 3:49 AM, Michael Paquier >> <[email protected]> wrote: >>> While going through numutils.c I found the following thing: >>> --- a/src/backend/utils/adt/numutils.c >>> +++ b/src/backend/utils/adt/numutils.c >>> @@ -136,7 +136,7 @@ pg_ltoa(int32 value, char *a) >>> * Avoid problems with the most negative integer not being representable >>> * as a positive integer. >>> */ >>> - if (value == (-2147483647 - 1)) >>> + if (value == PG_INT32_MIN) >>> { >>> memcpy(a, "-2147483648", 12); >>> return; >>> Attached is a patch. The interesting part is that pg_lltoa is not >>> missing the check on PG_INT64_MIN. > >> Committed. > > I am not very convinced that this is an improvement, because you took > what had been two hard-wired constants and replaced them with a symbol > and a hard-wired constant.This is more prone to break, not less so. > If there were a way to stringify PG_INT32_MIN's value for use in the > memcpy (which would then better be strcpy), then converting *both* > constants would be an improvement --- but otherwise I think this was > best left alone.
*shrug* I think it's kind of six of one, half a dozen of the other, but if you feel strongly about it, revert the patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
