On Mon, Feb 27, 2023 at 02:33:04PM +0000, gkokola...@pm.me wrote: > > > - Finally, the "Nothing to do in the default case" comment comes from > > > Michael's commit 5e73a6048: > > > > > > + /* > > > + * Custom and directory formats are compressed by default with gzip when > > > + * available, not the others. > > > + / > > > + if ((archiveFormat == archCustom || archiveFormat == archDirectory) && > > > + !user_compression_defined) > > > { > > > #ifdef HAVE_LIBZ > > > - if (archiveFormat == archCustom || archiveFormat == archDirectory) > > > - compressLevel = Z_DEFAULT_COMPRESSION; > > > - else > > > + parse_compress_specification(PG_COMPRESSION_GZIP, NULL, > > > + &compression_spec); > > > +#else > > > + / Nothing to do in the default case */ > > > #endif > > > - compressLevel = 0; > > > } > > > > > > As the comment says: for -Fc and -Fd, the compression is set to zlib, if > > > enabled, and when not otherwise specified by the user. > > > > > > Before 5e73a6048, this set compressLevel=0 for -Fp and -Ft, and when > > > zlib was unavailable. > > > > > > But I'm not sure why there's now an empty "#else". I also don't know > > > what "the default case" refers to. > > > > > > Maybe the best thing here is to move the preprocessor #if, since it's no > > > longer in the middle of a runtime conditional: > > > > > > #ifdef HAVE_LIBZ > > > + if ((archiveFormat == archCustom || archiveFormat == archDirectory) && > > > + !user_compression_defined) > > > + parse_compress_specification(PG_COMPRESSION_GZIP, NULL, > > > + &compression_spec); > > > #endif > > > > > > ...but that elicits a warning about "variable set but not used"... > > > > > > Not sure, I need to think about this a bit.
> /* Nothing to do for the default case when LIBZ is not available */ > is easier to understand. 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. The placement and phrasing of the comment makes no sense to me. -- Justin