Subtraction carry bug in xlog.c in 7.3 and 7.4

Started by J.R. Nieldover 22 years ago4 messages

J.R. Nield

jrnield@usol.com

over 22 years ago

2 attachment(s)

The attached patches against 7.3 and 7.4 fix a subtraction carry bug in
xlog.c.

--
John Nield
jrnield@usol.com

Attachments:

jrnield_003_7_3_subtract_carry.patchtext/plain; charset=ISO-8859-15; name=jrnield_003_7_3_subtract_carry.patchDownload

Index: pgsql-server-7_3/src/backend/access/transam/xlog.c
diff -c pgsql-server-7_3/src/backend/access/transam/xlog.c:1.1.1.1 pgsql-server-7_3/src/backend/access/transam/xlog.c:1.1.1.1.2.2
*** pgsql-server-7_3/src/backend/access/transam/xlog.c:1.1.1.1	Thu Jun 19 14:25:08 2003
--- pgsql-server-7_3/src/backend/access/transam/xlog.c	Thu Jun 19 18:21:54 2003
***************
*** 354,362 ****
  	  logSeg = (xlrp).xrecoff / XLogSegSize \
  	)
  #define XLByteToPrevSeg(xlrp, logId, logSeg)	\
! 	( logId = (xlrp).xlogid, \
! 	  logSeg = ((xlrp).xrecoff - 1) / XLogSegSize \
! 	)
  
  /*
   * Is an XLogRecPtr within a particular XLOG segment?
--- 354,366 ----
  	  logSeg = (xlrp).xrecoff / XLogSegSize \
  	)
  #define XLByteToPrevSeg(xlrp, logId, logSeg)	\
! 	do { \
! 		logId = (xlrp).xlogid; \
!         if ( (xlrp).xrecoff == 0 ) \
! 			{ logId--; logSeg = XLogSegsPerFile - 1; } \
! 		else \
! 			logSeg = ((xlrp).xrecoff - 1) / XLogSegSize; \
! 	} while (0)
  
  /*
   * Is an XLogRecPtr within a particular XLOG segment?
***************
*** 369,376 ****
  	 (xlrp).xrecoff / XLogSegSize == (logSeg))
  
  #define XLByteInPrevSeg(xlrp, logId, logSeg)	\
! 	((xlrp).xlogid == (logId) && \
! 	 ((xlrp).xrecoff - 1) / XLogSegSize == (logSeg))
  
  
  #define XLogFileName(path, log, seg)	\
--- 373,384 ----
  	 (xlrp).xrecoff / XLogSegSize == (logSeg))
  
  #define XLByteInPrevSeg(xlrp, logId, logSeg)	\
!   ( \
! 	((xlrp).xrecoff == 0)? \
! 		((xlrp).xlogid - 1 == (logId) && (XLogSegsPerFile - 1) == logSeg): \
! 		((xlrp).xlogid == (logId) &&  \
! 						((xlrp).xrecoff - 1) / XLogSegSize == (logSeg)) \
!   )
  
  
  #define XLogFileName(path, log, seg)	\

jrnield_004_7_4_subtract_carry.patchtext/x-patch; charset=ISO-8859-15; name=jrnield_004_7_4_subtract_carry.patchDownload

*** src/backend/access/transam/xlog.c.orig	Fri Jun 20 09:04:20 2003
--- src/backend/access/transam/xlog.c	Fri Jun 20 09:10:04 2003
***************
*** 354,362 ****
  	  logSeg = (xlrp).xrecoff / XLogSegSize \
  	)
  #define XLByteToPrevSeg(xlrp, logId, logSeg)	\
! 	( logId = (xlrp).xlogid, \
! 	  logSeg = ((xlrp).xrecoff - 1) / XLogSegSize \
! 	)
  
  /*
   * Is an XLogRecPtr within a particular XLOG segment?
--- 354,366 ----
  	  logSeg = (xlrp).xrecoff / XLogSegSize \
  	)
  #define XLByteToPrevSeg(xlrp, logId, logSeg)	\
! 	do { \
! 		logId = (xlrp).xlogid; \
!         if ( (xlrp).xrecoff == 0 ) \
! 			{ logId--; logSeg = XLogSegsPerFile - 1; } \
! 		else \
! 			logSeg = ((xlrp).xrecoff - 1) / XLogSegSize; \
! 	} while (0)
  
  /*
   * Is an XLogRecPtr within a particular XLOG segment?
***************
*** 369,376 ****
  	 (xlrp).xrecoff / XLogSegSize == (logSeg))
  
  #define XLByteInPrevSeg(xlrp, logId, logSeg)	\
! 	((xlrp).xlogid == (logId) && \
! 	 ((xlrp).xrecoff - 1) / XLogSegSize == (logSeg))
  
  
  #define XLogFileName(path, log, seg)	\
--- 373,384 ----
  	 (xlrp).xrecoff / XLogSegSize == (logSeg))
  
  #define XLByteInPrevSeg(xlrp, logId, logSeg)	\
!   ( \
! 	((xlrp).xrecoff == 0)? \
! 		((xlrp).xlogid - 1 == (logId) && (XLogSegsPerFile - 1) == logSeg): \
! 		((xlrp).xlogid == (logId) &&  \
! 						((xlrp).xrecoff - 1) / XLogSegSize == (logSeg)) \
!   )
  
  
  #define XLogFileName(path, log, seg)	\

Tom Lane

tgl@sss.pgh.pa.us

over 22 years ago

In reply to: J.R. Nield (#1)

Re: [HACKERS] Subtraction carry bug in xlog.c in 7.3 and 7.4

"J.R. Nield" <jrnield@usol.com> writes:

The attached patches against 7.3 and 7.4 fix a subtraction carry bug in
xlog.c.

This is simply a waste of cycles, because xrecoff can never be zero
(if it were, it'd be pointing at a page header, which is not a valid
record location). If you look around in the code, you'll note that
xrecoff==0 is actually used as an invalid-value flag in a couple of
contexts.

Can you point to anyplace where the behavior would change? If so
I think there's a deeper bug to fix.

regards, tom lane

Bruce Momjian

pgman@candle.pha.pa.us

over 22 years ago

In reply to: Tom Lane (#2)

Re: [HACKERS] Subtraction carry bug in xlog.c in 7.3 and 7.4

Should we add an Assert() to make it clear the current code is OK?

---------------------------------------------------------------------------

Tom Lane wrote:

"J.R. Nield" <jrnield@usol.com> writes:

The attached patches against 7.3 and 7.4 fix a subtraction carry bug in
xlog.c.

This is simply a waste of cycles, because xrecoff can never be zero
(if it were, it'd be pointing at a page header, which is not a valid
record location). If you look around in the code, you'll note that
xrecoff==0 is actually used as an invalid-value flag in a couple of
contexts.

Can you point to anyplace where the behavior would change? If so
I think there's a deeper bug to fix.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom Lane

tgl@sss.pgh.pa.us

over 22 years ago

In reply to: Bruce Momjian (#3)

Re: [HACKERS] Subtraction carry bug in xlog.c in 7.3 and 7.4

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Should we add an Assert() to make it clear the current code is OK?

A comment maybe, but not an assert.

regards, tom lane