Re: Two noncritical bugs of pg_waldump - Mailing list pgsql-hackers

Hmm..

At Thu, 27 Jan 2022 10:07:38 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> pg_waldump complains at the end in any case.  I noticed that the LSN
> it shows in the finish message is incorrect.  (I faintly thought that
> I posted about this but I didn't find it..)
> 
> > pg_waldump: fatal: error in WAL record at 0/15073F8: invalid record length at 0/1507470: wanted 24, got 0
> 
> xlogreader found the error at the record begins at 1507470, but
> pg_waldump tells that error happens at 15073F8, which is actually the
> beginning of the last sound record.

It is arguable, but the following is indisputable.

> If I give an empty file to the tool it complains as the follows.
> 
> > pg_waldump: fatal: could not read file "hoge": No such file or directory
> 
> No, the file exists.  The cause is it reads uninitialized errno to
> detect errors from the system call.  read(2) is defined to set errno
> always when it returns -1 and doesn't otherwise. Thus it seems to me
> that it is better to check that the return value is less than zero
> than to clear errno before the call to read().

So I post a patch contains only the indisputable part.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From f43cf938b0686dc406a1c81863a6415de1190b40 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Mon, 14 Feb 2022 18:11:19 +0900
Subject: [PATCH v1] Fix incorrect error handling of pg_waldump

search_directory of pg_waldump.c puts a wrong assumption about the
return value from read(2) and results in a bogus error message.  It
should check (r < 0) instead of (r != XLOG_BLCKSZ) to correctly detect
errors that %m work correctly on.
---
 src/bin/pg_waldump/pg_waldump.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index a6251e1a96..2340dc247b 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -222,15 +222,12 @@ search_directory(const char *directory, const char *fname)
                                      WalSegSz),
                             fname, WalSegSz);
         }
+        else if (r < 0)
+            fatal_error("could not read file \"%s\": %m",
+                        fname);
         else
-        {
-            if (errno != 0)
-                fatal_error("could not read file \"%s\": %m",
-                            fname);
-            else
-                fatal_error("could not read file \"%s\": read %d of %d",
-                            fname, r, XLOG_BLCKSZ);
-        }
+            fatal_error("could not read file \"%s\": read %d of %d",
+                        fname, r, XLOG_BLCKSZ);
         close(fd);
         return true;
     }
-- 
2.27.0