Thread: pg_restore: fix bogosity

pg_restore: fix bogosity

From
Neil Conway
Date:
The Coverity tool picked up some rather bizarre code in _tarGetHeader in
pg_backup_tar.c:

(1) The code doesn't initialize `sum', so the initial "does the checksum
match?" test is wrong.

(2) The loop that is intended to check for a "null block" just checks
the first byte of the tar block 512 times, rather than each of the 512
bytes one time (!), which I'm guessing was the intent.

Attached is a patch that I believe should implement what the author
intended. Barring any objections, I'll apply this to HEAD and back
branches today or tomorrow.

-Neil
Index: src/bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.47
diff -c -r1.47 pg_backup_tar.c
*** src/bin/pg_dump/pg_backup_tar.c    25 Jan 2005 22:44:31 -0000    1.47
--- src/bin/pg_dump/pg_backup_tar.c    20 Jun 2005 01:44:37 -0000
***************
*** 1155,1161 ****
      size_t        len;
      unsigned long ullen;
      off_t        hPos;
-     int            i;
      bool        gotBlock = false;

      while (!gotBlock)
--- 1155,1160 ----
***************
*** 1178,1184 ****
          hPos = ctx->tarFHpos;

          /* Read a 512 byte block, return EOF, exit if short */
!         len = _tarReadRaw(AH, &h[0], 512, NULL, ctx->tarFH);
          if (len == 0)            /* EOF */
              return 0;

--- 1177,1183 ----
          hPos = ctx->tarFHpos;

          /* Read a 512 byte block, return EOF, exit if short */
!         len = _tarReadRaw(AH, h, 512, NULL, ctx->tarFH);
          if (len == 0)            /* EOF */
              return 0;

***************
*** 1188,1207 ****
                           (unsigned long) len);

          /* Calc checksum */
!         chk = _tarChecksum(&h[0]);

          /*
!          * If the checksum failed, see if it is a null block. If so, then
!          * just try with next block...
           */
-
          if (chk == sum)
              gotBlock = true;
          else
          {
              for (i = 0; i < 512; i++)
              {
!                 if (h[0] != 0)
                  {
                      gotBlock = true;
                      break;
--- 1187,1208 ----
                           (unsigned long) len);

          /* Calc checksum */
!         chk = _tarChecksum(h);
!         sscanf(&h[148], "%8o", &sum);

          /*
!          * If the checksum failed, see if it is a null block. If so,
!          * silently continue to the next block.
           */
          if (chk == sum)
              gotBlock = true;
          else
          {
+             int            i;
+
              for (i = 0; i < 512; i++)
              {
!                 if (h[i] != 0)
                  {
                      gotBlock = true;
                      break;
***************
*** 1213,1219 ****
      sscanf(&h[0], "%99s", tag);
      sscanf(&h[124], "%12lo", &ullen);
      len = (size_t) ullen;
-     sscanf(&h[148], "%8o", &sum);

      {
          char        buf[100];
--- 1214,1219 ----

Re: pg_restore: fix bogosity

From
Neil Conway
Date:
Neil Conway wrote:
> Attached is a patch that I believe should implement what the author
> intended.

Applied.

-Neil