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,

Attachment: signature.asc
Description: PGP signature

Reply via email to