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 ----
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

               http://www.postgresql.org/docs/faq

Reply via email to