On Tue, Apr 11, 2023 at 07:41:11PM -0500, Justin Pryzby wrote: > Maybe I would write it as: "if zlib is unavailable, default to no > compression". But I think that's best done in the leading comment, and > not inside an empty preprocessor #else. > > I was hoping Michael would comment on this.
(Sorry for the late reply, somewhat missed that.) > The placement and phrasing of the comment makes no sense to me. Yes, this comment gives no value as it stands. I would be tempted to follow the suggestion to group the whole code block in a single ifdef, including the check, and remove this comment. Like the attached perhaps? -- Michael
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 967ced4eed..b7dda5ab27 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -753,16 +753,14 @@ main(int argc, char **argv) * Custom and directory formats are compressed by default with gzip when * available, not the others. */ +#ifdef HAVE_LIBZ if ((archiveFormat == archCustom || archiveFormat == archDirectory) && !user_compression_defined) { -#ifdef HAVE_LIBZ parse_compress_specification(PG_COMPRESSION_GZIP, NULL, &compression_spec); -#else - /* Nothing to do in the default case */ -#endif } +#endif /* * If emitting an archive format, we always want to emit a DATABASE item,
signature.asc
Description: PGP signature