Re: Change atoi to strtol in same place

From: Joe Nelson <joe(at)begriffs(dot)com>
To: Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, surafel3000(at)gmail(dot)com, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: Change atoi to strtol in same place
Date: 2019-10-07 00:21:50
Message-ID: 20191007002150.GD68117@begriffs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ashutosh Sharma wrote:
> One suggestion: The start value for port number is set to 1, however
> it seems like the port number that falls in the range of 1-1023 is
> reserved and can't be used. So, is it possible to have the start value
> as 1024 instead of 1 ?

Good idea -- changed it. I also created macros FE_UTILS_PORT_{MIN,MAX}
so the range can be updated in one place for all utilities.

> Further, I encountered one syntax error (INT_MAX undeclared) as the
> header file "limits.h" has not been included in postgres_fe.h or
> option.h

Oops. Added limits.h now in option.h. The Postgres build accidentally
worked on my system without explicitly including this header because
__has_builtin(__builtin_isinf) is true for me so src/include/port.h
pulled in math.h with an #if which pulled in limits.h.

> Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> > The wording is a bit strange. How about something like pg_standy:
> > invalid argument to -k: %s

I updated the error messages in pg_standby.

> > > *error = psprintf(_("could not parse '%s' as integer"), str);
> >
> > ... except that they would rather be more explicit about what the
> > problem is. "insufficient digits" or "extraneous character", etc.

Sadly pg_strtoint64 returns the same error code for both cases. So we
could either petition for more detailed errors in the thread for that
other patch, or examine the string ourselves to check. Maybe it's not
needed since "could not parse 'abc' as integer" kind of does show the
problem.

> > I hope that no callers would like to have the messages not translated,
> > because that seems like it would become a mess.

That's true... I think it should be OK though, since we return the
pg_strtoint_status so callers can inspect that rather than relying on certain
words being in the error string. I'm guessing the translated string would be
most appropriate for end users.

--
Joe Nelson https://begriffs.com

Attachment Content-Type Size
atoi-to-strtol-v5.patch text/x-patch 27.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2019-10-07 00:55:23 adding partitioned tables to publications
Previous Message Andrew Gierth 2019-10-06 21:39:10 Re: v12 and pg_restore -f-