Unnecessary static variable?

Started by Antonin Houskaalmost 8 years ago5 messages

ah@cybertec.at

almost 8 years ago

While reading XLogPageRead() I was surprised that readLen variable is set but
not used in the read() call. Then I realized that it's declared static
although no other function uses it. Maybe it was used earlier to exit early if
sufficient amount of data was already read? I think this case is now handled
by the calling function xlogreader.c:ReadPageInternal().

I suggest to make the variable local:

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
new file mode 100644
index e42b828..c3267f5
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*************** static XLogSegNo openLogSegNo = 0;
*** 776,792 ****
  static uint32 openLogOff = 0;

/*
! * These variables are used similarly to the ones above, but for reading
! * the XLOG. Note, however, that readOff generally represents the offset
! * of the page just read, not the seek position of the FD itself, which
! * will be just past that page. readLen indicates how much of the current
! * page has been read into readBuf, and readSource indicates where we got
! * the currently open file from.
*/
static int readFile = -1;
static XLogSegNo readSegNo = 0;
static uint32 readOff = 0;
- static uint32 readLen = 0;
static XLogSource readSource = 0; /* XLOG_FROM_* code */

  /*
--- 776,790 ----
  static uint32 openLogOff = 0;

/*
! * These variables are used similarly to the ones above, but for reading the
! * XLOG. Note, however, that readOff generally represents the offset of the
! * page just read, not the seek position of the FD itself, which will be just
! * past that page. readSource indicates where we got the currently open file
! * from.
*/
static int readFile = -1;
static XLogSegNo readSegNo = 0;
static uint32 readOff = 0;
static XLogSource readSource = 0; /* XLOG_FROM_* code */

  /*
*************** XLogPageRead(XLogReaderState *xlogreader
*** 11556,11561 ****
--- 11554,11560 ----
  	(XLogPageReadPrivate *) xlogreader->private_data;
  	int			emode = private->emode;
  	uint32		targetPageOff;
+ 	uint32		readLen;
  	XLogSegNo	targetSegNo PG_USED_FOR_ASSERTS_ONLY;

XLByteToSeg(targetPagePtr, targetSegNo, wal_segment_size);
*************** retry:
*** 11603,11609 ****
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = 0;

  			return -1;
--- 11602,11607 ----
*************** retry:
*** 11618,11626 ****

  	/*
  	 * If the current segment is being streamed from master, calculate how
! 	 * much of the current page we have received already. We know the
! 	 * requested record has been received, but this is for the benefit of
! 	 * future calls, to allow quick exit at the top of this function.
  	 */
  	if (readSource == XLOG_FROM_STREAM)
  	{
--- 11616,11622 ----

/*
* If the current segment is being streamed from master, calculate how
! * much of the current page we have received already.
*/
if (readSource == XLOG_FROM_STREAM)
{
*************** retry:
*** 11648,11654 ****
}

pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
! if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
char fname[MAXFNAMELEN];

--- 11644,11650 ----
  	}

pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
! if (read(readFile, readBuf, readLen) != readLen)
{
char fname[MAXFNAMELEN];

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com