On Mon, Jul 20, 2026 at 7:12 PM Mahendra Singh Thalor
<[email protected]> wrote:
> Here, I am attaching 3 patches for the review.

Thanks for updating the patches!

+ check_mut_excl_opts(opts->cparams.dbname, "-d/--dbname",
+ opts->filename, "-f/--file");
...
+ /* --dbname and --restrict-key are incompatible */
+ check_mut_excl_opts(opts->cparams.dbname, "-d/--dbname",
+ opts->restrict_key, "--restrict-key");

opts->cparams.dbname, opts->filename, and opts->restrict_key are pointers.
Passing them to check_mut_excl_opts() and reading them back as int is
undefined behavior. Shouldn't these calls instead be written as:

    check_mut_excl_opts(opts->cparams.dbname != NULL, "-d/--dbname",
                      opts->filename != NULL, "-f/--file");

    check_mut_excl_opts(opts->cparams.dbname != NULL, "-d/--dbname",
                      opts->restrict_key != NULL, "--restrict-key");


I'm wondering about backpatching. Patch 0003 fixes undefined behavior
introduced by check_mut_excl_opts(), so it seems appropriate to
backpatch it to v19. Patches 0001 and 0002 are mostly refactoring,
but they restore the v19-era cleanup that was lost when the pg_dumpall
non-text-format support was reverted, so backpatching those to v19 also
seems OK to me. Thoughts?

If we commit these, I think it would make sense to merge them into
a single commit.

Regards,

-- 
Fujii Masao


Reply via email to