Horton

Re: pg_verifybackup: TAR format backup verification

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amul Sul <sulamul@gmail.com>, Sravan Kumar <sravanvcybage@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-10-01T14:39:51Z
Lists: pgsql-hackers

Attachments

On Mon, Sep 30, 2024 at 6:01 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > But then how do you think we should print
> > that? Cast to unsigned long long and use %llu?
>
> Our two standard solutions are to do that or to use UINT64_FORMAT.
> But UINT64_FORMAT is problematic in translatable strings because
> then the .po files would become platform-specific, so long long
> is what to use in that case.  For a non-translated format string
> you can do either.

Here is an attempt at cleaning this up. I'm far from convinced that
it's fully correct; my local compiler (clang version 15.0.7) doesn't
seem fussed about conflating size_t with uint64, not even with -Wall
-Werror. I don't suppose you have a fussier compiler locally that you
can use to test this?

> > I don't understand what you think the widely-used, better solution is
> > here.
>
> What we just said above.

Respectfully, if you'd just said in your first email about this "I
understand that you were trying to be consistent with a format string
somewhere else, but I don't think that's a good reason to do it this
way, so please use %llu and insert a cast," I would have just said
"fine, no problem" and I wouldn't have been irritated at all. But you
seem determined to deny the existence of the argument that I made
instead of just disagreeing with it, and that's actually pretty
frustrating. I feel like you've wasted my time and your own to no
purpose, and made me feel stupid in the process, over something that
barely even matters. Anyone who has a control file bigger than 2GB has
... a lot of issues.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

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.