Unnecessary static variable? - Mailing list pgsql-hackers

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