Re: pg_amcheck: Fix block number parsing on command line

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_amcheck: Fix block number parsing on command line
Date: 2021-07-22 16:18:51
Message-ID: E2D4E4A5-E2CA-4B1A-9558-73C7EF27E61C@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jul 22, 2021, at 7:56 AM, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> Please check that it's up to speed.
> <0001-pg_amcheck-Fix-block-number-parsing-on-command-line.patch>

This looks correct to me. Thanks for the fix.

Your use of strtoul compares favorably to that in pg_resetwal in that you are checking errno and it is not. The consequence is:

bin % ./pg_resetwal/pg_resetwal -e 1111111111111111111111111111111111111111111111111111
pg_resetwal: error: transaction ID epoch (-e) must not be -1
bin % ./pg_resetwal/pg_resetwal -e junkstring
pg_resetwal: error: invalid argument for option -e
Try "pg_resetwal --help" for more information.

Unless people are relying on this behavior, I would think pg_resetwal should complain of an invalid argument for both of those, rather than complaining about -1. That's not to do with this patch, but if we're tightening up the use of strtol in frontend tools, maybe we can use the identical logic in both places.


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2021-07-22 16:20:35 Re: Rename of triggers for partitioned tables
Previous Message Pavel Stehule 2021-07-22 16:17:01 Re: proposal: enhancing plpgsql debug API - returns text value of variable content