On Mon, Jan 10, 2022 at 12:19:28AM +0800, Wei Sun wrote: > Hi, > > Some time ago,the following patch clean up error handling in pg_basebackup's > walmethods.c. > https://github.com/postgres/postgres/commit/248c3a9 > > This patch keep the error state in the DirectoryMethodData struct, > in most functions, the lasterrno is set correctly, but in function > dir_existsfile(), > the lasterrno is not set when the file fails to open. > > If this is a correction omission, I think this patch can fix this. > > Cheers
> diff --git a/src/bin/pg_basebackup/walmethods.c > b/src/bin/pg_basebackup/walmethods.c > index f74bd13..35cf5a8 100644 > --- a/src/bin/pg_basebackup/walmethods.c > +++ b/src/bin/pg_basebackup/walmethods.c > @@ -580,7 +580,10 @@ dir_existsfile(const char *pathname) > > fd = open(tmppath, O_RDONLY | PG_BINARY, 0); > if (fd < 0) > + { > + dir_data->lasterrno = errno; > return false; > + } > close(fd); > return true; > } Looking at this, the function is used to check if something exists, and return a boolean. I am not sure it is helpful to also return a ENOENT in the lasterrno status field. It might be useful to set lasterrno if the open fails and it is _not_ ENOENT. -- Bruce Momjian <br...@momjian.us> https://momjian.us EDB https://enterprisedb.com Indecision is a decision. Inaction is an action. Mark Batterson