public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: Chao Li <[email protected]>
Cc: Álvaro Herrera <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Postgres hackers <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Subject: Re: Cleanup shadows variable warnings, round 1
Date: Wed, 22 Apr 2026 16:32:25 +1000
Message-ID: <CAHut+PuK3t_o63=x4N26sMKBF1koxCJdzQ75zuc-igyskigvtQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

Hi Chao-San.

A couple of comments for your v1-0001 cleanup patch.

======
src/bin/pg_dump/pg_dumpall.c

I guess you were just making the minimal changes, but I thought
parseDumpFormat could have been simplified more by removing that local
variable entirely.

BEFORE
if (pg_strcasecmp(format, "c") == 0)
  archFormat = archCustom;
else if (pg_strcasecmp(format, "custom") == 0)
  archFormat = archCustom;
else if (pg_strcasecmp(format, "d") == 0)
  archFormat = archDirectory;
else if (pg_strcasecmp(format, "directory") == 0)
  archFormat = archDirectory;
else if (pg_strcasecmp(format, "p") == 0)
  archFormat = archNull;
else if (pg_strcasecmp(format, "plain") == 0)
  archFormat = archNull;
else if (pg_strcasecmp(format, "t") == 0)
  archFormat = archTar;
else if (pg_strcasecmp(format, "tar") == 0)
  archFormat = archTar;

SUGGESTION
if (pg_strcasecmp(format, "c") == 0 ||
  pg_strcasecmp(format, "custom") == 0)
  return archCustom;

if (pg_strcasecmp(format, "d") == 0 ||
  pg_strcasecmp(format, "directory") == 0)
  return archDirectory;

if (pg_strcasecmp(format, "p") == 0 ||
  pg_strcasecmp(format, "plain") == 0)
  return archNull;

if (pg_strcasecmp(format, "t") == 0 ||
  pg_strcasecmp(format, "tar") == 0)
  return archTar;

======
src/bin/psql/describe.c

I know you were addressing only "new" issues, but it seemed a bit
strange to fix only this one when there was the same issue earlier
(~line 1780) in the same function.

if (tableinfo.relkind == RELKIND_SEQUENCE)
{
 PGresult   *result = NULL;
 printQueryOpt myopt = pset.popt;

======
Kind Regards,
Peter Smith.
Fujitsu Australia





view thread (30+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Cleanup shadows variable warnings, round 1
  In-Reply-To: <CAHut+PuK3t_o63=x4N26sMKBF1koxCJdzQ75zuc-igyskigvtQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox