Re: Change atoi to strtol in same place

From: Joe Nelson <joe(at)begriffs(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Surafel Temesgen <surafel3000(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Change atoi to strtol in same place
Date: 2019-07-24 04:02:37
Message-ID: 20190724040237.GB64205@begriffs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Surafel Temesgen wrote:
> > we use atoi for user argument processing in same place which return
> > zero for both invalid input and input value zero. [...] in same
> > place where we accept zero as valued input it case a problem by
> > preceding for invalid input as input value zero. strtol which can
> > handle invalid input

Not only that, but atoi causes Undefined Behavior on erroneous input.
The C99 standard says this:

7.20.1 Numeric conversion functions
The functions atof, atoi, atol, and atoll need not affect the value of the
integer expression errno on an error. If the value of the result cannot be
represented, the behavior is undefined.

Tomas Vondra wrote:
> This seems to have bit-rotted (due to minor changes to pg_basebackup).
> Can you fix that and post an updated version?

I adjusted the patch to apply cleanly on a0555ddab9.

> In general, I think it's a good idea to fix those places, but I wonder
> if we need to change the error messages too.

I'll leave that decision for the community to debate. I did, however,
remove the newlines for the new error messages being passed to
pg_log_error().

As discussed in message [0], the logging functions in common/logging.c
now contain an assertion that messages do not end in newline:

Assert(fmt[strlen(fmt) - 1] != '\n');

(in pg_log_error via pg_log_generic via pg_log_generic_v)

I also added limits.h to some places it was missing, so the patch would
build.

0: https://postgr.es/m/6a609b43-4f57-7348-6480-bd022f924310@2ndquadrant.com

Attachment Content-Type Size
atoi-to-strtol-v2.patch text/x-patch 14.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2019-07-24 04:03:34 Re: block-level incremental backup
Previous Message Thomas Munro 2019-07-24 03:52:40 Re: On the stability of TAP tests for LDAP