Postgres Pro
Downloads
Home   >   mailing lists

Thread: Silly coding in pgcrypto

Silly coding in pgcrypto

From
Marko Tiikkaja
Date:
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
Attachment

Re: Silly coding in pgcrypto

From
Noah Misch
Date:
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?

Re: Silly coding in pgcrypto

From
Tomas Vondra
Date:
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

Re: Silly coding in pgcrypto

From
Marko Tiikkaja
Date:
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

Re: Silly coding in pgcrypto

From
Noah Misch
Date:
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.