Silly coding in pgcrypto

Started by Marko Tiikkajaabout 11 years ago5 messages
#1Marko Tiikkaja
marko@joh.to
1 attachment(s)

Hi,

Patch attached to fix some sillyness in pgp_expect_packet_end() and
pgp_skip_packet(). Will add to the next commitfest unless someone picks
this one up before that.

.marko

Attachments:

pgcrypto_sillyness.patchtext/plain; charset=UTF-8; name=pgcrypto_sillyness.patch; x-mac-creator=0; x-mac-type=0Download
*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 1069,1075 **** pgp_skip_packet(PullFilter *pkt)
  
  	while (res > 0)
  		res = pullf_read(pkt, 32 * 1024, &tmp);
! 	return res < 0 ? res : 0;
  }
  
  /*
--- 1069,1075 ----
  
  	while (res > 0)
  		res = pullf_read(pkt, 32 * 1024, &tmp);
! 	return res;
  }
  
  /*
***************
*** 1078,1096 **** pgp_skip_packet(PullFilter *pkt)
  int
  pgp_expect_packet_end(PullFilter *pkt)
  {
! 	int			res = 1;
  	uint8	   *tmp;
  
! 	while (res > 0)
  	{
! 		res = pullf_read(pkt, 32 * 1024, &tmp);
! 		if (res > 0)
! 		{
! 			px_debug("pgp_expect_packet_end: got data");
! 			return PXE_PGP_CORRUPT_DATA;
! 		}
  	}
! 	return res < 0 ? res : 0;
  }
  
  int
--- 1078,1093 ----
  int
  pgp_expect_packet_end(PullFilter *pkt)
  {
! 	int			res;
  	uint8	   *tmp;
  
! 	res = pullf_read(pkt, 32 * 1024, &tmp);
! 	if (res > 0)
  	{
! 		px_debug("pgp_expect_packet_end: got data");
! 		return PXE_PGP_CORRUPT_DATA;
  	}
! 	return res;
  }
  
  int
#2Noah Misch
noah@leadboat.com
In reply to: Marko Tiikkaja (#1)
Re: Silly coding in pgcrypto

On Sun, Nov 02, 2014 at 05:10:25AM +0100, Marko Tiikkaja wrote:

*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 1069,1075 **** pgp_skip_packet(PullFilter *pkt)

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res < 0 ? res : 0;
}

/*
--- 1069,1075 ----

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res;

Why is the old code silly and the new code correct?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tomas Vondra
tv@fuzzy.cz
In reply to: Noah Misch (#2)
Re: Silly coding in pgcrypto

On 2.11.2014 22:34, Noah Misch wrote:

On Sun, Nov 02, 2014 at 05:10:25AM +0100, Marko Tiikkaja wrote:

*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 1069,1075 **** pgp_skip_packet(PullFilter *pkt)

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res < 0 ? res : 0;
}

/*
--- 1069,1075 ----

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res;

Why is the old code silly and the new code correct?

The loop only terminates when (res > 0) is false, i.e. (res <= 0), which
makes the expression after the return statement pointless:

(res == 0) -> 0
(res < 0) -> res

So it's 'res' all the time.

Tomas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Marko Tiikkaja
marko@joh.to
In reply to: Noah Misch (#2)
Re: Silly coding in pgcrypto

On 11/2/14, 10:34 PM, Noah Misch wrote:

On Sun, Nov 02, 2014 at 05:10:25AM +0100, Marko Tiikkaja wrote:

*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 1069,1075 **** pgp_skip_packet(PullFilter *pkt)

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res < 0 ? res : 0;
}

/*
--- 1069,1075 ----

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res;

Why is the old code silly and the new code correct?

When the loop terminates, res can only be <= 0. If res is less than 0,
res is returned. In all other cases (i.e. when res == 0), 0 is
returned. The ternary expression is completely unnecessary.

.marko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Noah Misch
noah@leadboat.com
In reply to: Marko Tiikkaja (#4)
Re: Silly coding in pgcrypto

On Sun, Nov 02, 2014 at 10:53:27PM +0100, Marko Tiikkaja wrote:

On 11/2/14, 10:34 PM, Noah Misch wrote:

On Sun, Nov 02, 2014 at 05:10:25AM +0100, Marko Tiikkaja wrote:

*** a/contrib/pgcrypto/pgp-decrypt.c
--- b/contrib/pgcrypto/pgp-decrypt.c
***************
*** 1069,1075 **** pgp_skip_packet(PullFilter *pkt)

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res < 0 ? res : 0;
}

/*
--- 1069,1075 ----

while (res > 0)
res = pullf_read(pkt, 32 * 1024, &tmp);
! return res;

Why is the old code silly and the new code correct?

When the loop terminates, res can only be <= 0. If res is less than 0, res
is returned. In all other cases (i.e. when res == 0), 0 is returned. The
ternary expression is completely unnecessary.

Quite so. Committed.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers