Thread: Replace some %llu remnants in the tree
Hi all, While hacking a different patch, I've noticed that a couple of %llu did not get the PRIu64 call in the AIO code, and I don't see why we could not switch them. These have been introduced in commits that got into the tree after Peter's 15a79c73111f. A couple remain even after the attached, which have been left off by Peter for the same reasons as the ones I am guessing here: - launch_backend.c for paramHandle, does not seem worth it. - ecpglib/execute.c, when storing some input, which does not seem worth bothering either. - reconstruct.c, offset handling. - pg_backup_tar.c, ftello() result and some consistency with the surroundings. - pg_verifybackup.c and astreamer_verify.c, for Sizes. That's not necessarily mandatory for v18, for sure, but as this is new code we could as well clean it up before forking the next stable branch. Comments or opinions? -- Michael
Attachment
On 09.06.25 05:59, Michael Paquier wrote: > While hacking a different patch, I've noticed that a couple of %llu > did not get the PRIu64 call in the AIO code, and I don't see why we > could not switch them. These have been introduced in commits that got > into the tree after Peter's 15a79c73111f. Looks good. > That's not necessarily mandatory for v18, for sure, but as this is new > code we could as well clean it up before forking the next stable > branch. Agree this should go into v18. > A couple remain even after the attached, which have been left off by > Peter for the same reasons as the ones I am guessing here: > - launch_backend.c for paramHandle, does not seem worth it. According to Microsoft documentation, LONG_PTR is __int64, and so using PRId64 would seem to be appropriate. However, I wonder whether it wouldn't be simpler to print the original paramHandle either as %p or cast to uintptr_t, to avoid having two separate code paths. > - ecpglib/execute.c, when storing some input, which does not seem > worth bothering either. These are dealing with actual long long int values, to using %lld/%llu seems appropriate. > - reconstruct.c, offset handling. > - pg_backup_tar.c, ftello() result and some consistency with the > surroundings. > - pg_verifybackup.c and astreamer_verify.c, for Sizes. These are using off_t or pg_off_t, for which there is no format placeholder, so you have to cast it to something.
On Thu, Jun 12, 2025 at 07:16:37AM +0900, Michael Paquier wrote: > On Wed, Jun 11, 2025 at 09:58:00AM +0200, Peter Eisentraut wrote: >> On 09.06.25 05:59, Michael Paquier wrote: >>> That's not necessarily mandatory for v18, for sure, but as this is new >>> code we could as well clean it up before forking the next stable >>> branch. >> >> Agree this should go into v18. > > Thanks for the review. Adding the RMT in CC for more comments. Would > you be OK with the patch added to v18? The answer is probably yes, > but let's ask anyway. Seems fine to me. -- nathan