From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Amul Sul <sulamul(at)gmail(dot)com>, Sravan Kumar <sravanvcybage(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_verifybackup: TAR format backup verification |
Date: | 2024-09-28 22:59:02 |
Message-ID: | 1115881.1727564342@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2024-09-29 00:02:21 | Re: [PATCH] Add native windows on arm64 support |
Previous Message | Alena Rybakina | 2024-09-28 21:22:28 | Re: Vacuum statistics |