Re: [HACKERS] Silly coding in pgcrypto

2014-11-02 Thread Noah Misch 
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

Re: [HACKERS] Silly coding in pgcrypto

2014-11-02 Thread Marko Tiikkaja 

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

Re: [HACKERS] Silly coding in pgcrypto

2014-11-02 Thread Tomas Vondra 
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

Re: [HACKERS] Silly coding in pgcrypto

2014-11-02 Thread Noah Misch 
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

[HACKERS] Silly coding in pgcrypto

2014-11-01 Thread Marko Tiikkaja 

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
*** 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

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