Hi, When I was trying to understand what was going on in thread [1], I came across posix_fadvise() and was wondering how it is used. So I searched for it in the source tree and spotted two related issues.
1
```
rc = posix_fadvise(fd, offset, nbytes, POSIX_FADV_DONTNEED);
if (rc != 0)
{
/* don't error out, this is just a performance
optimization */
ereport(WARNING,
(errcode_for_file_access(),
errmsg("could not flush dirty data:
%m")));
}
```
This code uses %m to report an error from posix_fadvise(). However,
posix_fadvise() doesn't set errno on failure. Instead, it returns the error
code directly. See [2].
Therefore, both %m may use an unrelated errno value. As a simple fix, we can
assign rc to errno before calling ereport().
2
```
/*
* FilePrefetch - initiate asynchronous read of a given range of the file.
*
* Returns 0 on success, otherwise an errno error code (like posix_fadvise()).
*
* posix_fadvise() is the simplest standardized interface that accomplishes
* this.
*/
int
FilePrefetch(File file, pgoff_t offset, pgoff_t amount, uint32 wait_event_info)
{
...
returnCode = FileAccess(file);
if (returnCode < 0)
return returnCode;
```
FilePrefetch() is intended to return an error code, just like posix_fadvise().
However, FileAccess() returns -1 and sets errno on failure:
```
/* returns 0 on success, -1 on re-open failure (with errno set) */
static int
FileAccess(File file)
```
So here we should return errno instead of returnCode.
The attached patch fixes these two issues. As these are eye-caught issues, I
don't have a repro for either of them. I don’t even think my local build
exercises the affected code paths. So I'll put the patch on the CommitFest and
let CFBot test it.
[1]
https://postgr.es/m/CAHg+QDf=eFFnOvmaPJQrXNiXaTOhhJhvO=DzmVUi=vgfrh5...@mail.gmail.com
[2] https://man7.org/linux/man-pages/man3/posix_fadvise.3p.html
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v1-0001-Fix-error-handling-in-pg_flush_data-and-FilePrefe.patch
Description: Binary data
