Обсуждение: errno clobbering in reorderbuffer
While researching a customer issue with BDR I noticed that one ereport() call happens after clobbering errno, leading to the wrong strerror being reported. This patch fixes it by saving before calling CloseTransientFile and restoring afterwards. I also threw in a missing errcode I noticed while looking for similar problems in the same file. This is to backpatch to 9.4. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: > While researching a customer issue with BDR I noticed that one ereport() > call happens after clobbering errno, leading to the wrong strerror being > reported. This patch fixes it by saving before calling > CloseTransientFile and restoring afterwards. > > I also threw in a missing errcode I noticed while looking for similar > problems in the same file. > > This is to backpatch to 9.4. Makes sense - you're doing that or shall I?
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> While researching a customer issue with BDR I noticed that one ereport()
> call happens after clobbering errno, leading to the wrong strerror being
> reported. This patch fixes it by saving before calling
> CloseTransientFile and restoring afterwards.
> I also threw in a missing errcode I noticed while looking for similar
> problems in the same file.
Looks sane to me.
regards, tom lane
Hi,
On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote:
> if (write(fd, rb->outbuf, ondisk->size) != ondisk->size)
> {
> + int save_errno = errno;
> +
> CloseTransientFile(fd);
> + errno = save_errno;
> ereport(ERROR,
> (errcode_for_file_access(),
> errmsg("could not write to data file for XID %u: %m",
Independent of this specific case I kinda wish we had a better way to
deal with exactly this pattern. I even wonder whether having a close
variant not clobbering errno might be worthwhile.
Andres Freund wrote: > On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: > > While researching a customer issue with BDR I noticed that one ereport() > > call happens after clobbering errno, leading to the wrong strerror being > > reported. This patch fixes it by saving before calling > > CloseTransientFile and restoring afterwards. > > > > I also threw in a missing errcode I noticed while looking for similar > > problems in the same file. > > > > This is to backpatch to 9.4. > > Makes sense - you're doing that or shall I? I am, if you don't mind. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On August 18, 2016 7:21:03 PM PDT, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >Andres Freund wrote: >> On 2016-08-18 19:06:02 -0300, Alvaro Herrera wrote: >> > While researching a customer issue with BDR I noticed that one >ereport() >> > call happens after clobbering errno, leading to the wrong strerror >being >> > reported. This patch fixes it by saving before calling >> > CloseTransientFile and restoring afterwards. >> > >> > I also threw in a missing errcode I noticed while looking for >similar >> > problems in the same file. >> > >> > This is to backpatch to 9.4. >> >> Makes sense - you're doing that or shall I? > >I am, if you don't mind. Not at all, thanks. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Andres Freund wrote: > Not at all, thanks. Done, thanks both for the look-over. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services