From: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Postgres hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Incorrect errno used with %m for backend code |
Date: | 2018-06-22 07:30:45 |
Message-ID: | CAE9k0P=jJBfZkjSNWi0CqSC7JgCZBLzetFZ7Ng9mF7zoL0pv0A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
It seems like in case of few system calls for e.g. write system call,
errno is not set even if the number of bytes written is smaller than
the bytes requested and for such cases we explicitly set an errno to
ENOSPC. Something like this,
/*
* if write didn't set errno, assume problem is no disk space
*/
errno = save_errno ? save_errno : ENOSPC;
Shouldn't we do similar handling in your patch as well or please let
me know if i am missing something here.
On Fri, Jun 22, 2018 at 11:45 AM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> Hi all,
>
> I have been reviewing the use of errno in the backend code when %m is
> used in the logs, and found more places than when I looked at improving
> the error messages for read() calls which bloat the errno value because
> of intermediate system calls. As the problem is separate and should be
> back-patched, I have preferred beginning a new thread.
>
> A couple of places use CloseTransientFile without bothering much that
> this can overwrite errno. I was tempted in keeping errno saved and kept
> if set to a non-zero value, but refrained from doing so as some callers
> may rely on the existing behavior, and the attached shows better the
> intention.
>
> Attached is a patch with everything I have spotted. Any opinions or
> thoughts in getting this back-patched?
>
> Thanks,
> --
> Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2018-06-22 07:33:12 | Re: Possible bug in logical replication. |
Previous Message | Craig Ringer | 2018-06-22 07:24:15 | Re: Continue work on changes to recovery.conf API |