Re: pg_verifybackup: TAR format backup verification

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amul Sul <sulamul@gmail.com>, Sravan Kumar <sravanvcybage@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-09-28T22:59:02Z
Lists: pgsql-hackers
The 32-bit buildfarm animals don't like this too much [1]:

astreamer_verify.c: In function 'member_verify_checksum':
astreamer_verify.c:297:68: error: format '%zu' expects argument of type 'size_t', but argument 6 has type 'uint64' {aka 'long long unsigned int'} [-Werror=format=]
  297 |        "file \\"%s\\" in \\"%s\\" should contain %zu bytes, but read %zu bytes",
      |                                                                  ~~^
      |                                                                    |
      |                                                                    unsigned int
      |                                                                  %llu
  298 |        m->pathname, mystreamer->archive_name,
  299 |        m->size, mystreamer->checksum_bytes);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~                          
      |                           |
      |                           uint64 {aka long long unsigned int}

Now, manifest_file.size is in fact a size_t, so %zu is the correct
format spec for it.  But astreamer_verify.checksum_bytes is declared
uint64.  This leads me to question whether size_t was the correct
choice for manifest_file.size.  If it is, is it OK to use size_t
for checksum_bytes as well?  If not, your best bet is likely
to write %lld and add an explicit cast to long long, as we do in
dozens of other places.  I see that these messages are intended to be
translatable, so INT64_FORMAT is not usable here.

Aside from that, I'm unimpressed with expending a five-line comment
at line 376 to justify casting control_file_bytes to int, when you
could similarly cast it to long long, avoiding the need to justify
something that's not even in tune with project style.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2024-09-28%2006%3A03%3A02



Commits

  1. Adjust json_manifest_per_file_callback API in one more place.

  2. File size in a backup manifest should use uint64, not size_t.

  3. Fix some pg_verifybackup issues reported by Coverity.

  4. pg_verifybackup: Move some declarations to new pg_verifybackup.h

  5. pg_verifybackup: Move skip_checksums into verifier_context.

  6. Improve file header comments for astramer code.

  7. Move astreamer (except astreamer_inject) to fe_utils.

  8. Move recovery injector astreamer to a separate header file.

  9. Rename bbstreamer to astreamer.