Re: Small patch for pg_basebackup argument parsing

On Friday, April 14, 2017 8:59:03 AM CEST Robert Haas wrote:
> On Fri, Apr 14, 2017 at 2:32 AM, Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info>
wrote:
> > On Friday, April 14, 2017 8:44:37 AM CEST Michael Paquier wrote:
> >> On Fri, Apr 14, 2017 at 6:32 AM, Pierre Ducroquet <p(dot)psql(at)pinaraf(dot)info>
> >
> > wrote:
> >> > Yesterday while doing a few pg_basebackup, I realized that the integer
> >> > parameters were not properly checked against invalid input.
> >> > It is not a critical issue, but this could be misleading for an user
> >> > who
> >> > writes -z max or -s 0.5…
> >> > I've attached the patch to this mail. Should I add it to the next
> >> > commit
> >> > fest or is it not needed for such small patches ?
> >>
> >> A call to atoi is actually equivalent to strtol with the rounding:
> >> (int)strtol(str, (char **)NULL, 10);
> >> So I don't think this is worth caring.
> >
> > The problem with atoi is that it simply ignores any invalid input and
> > returns 0 instead.
> > That's why I did this patch, because I did a typo when calling
> > pg_basebackup and was not warned for an invalid input.
>
> I agree. I think it would be worth going through and cleaning up
> every instance of this in the source tree.

Hi

Following your advice, I went through the source tree and cleaned up most
instances of that pattern.
I have attached the corresponding patch to this mail.
If you think this patch is indeed interesting, what would be the next way to
have it reviewed ?

Thanks

Pierre