| Lists: | pgsql-hackers |
|---|
| From: | Marko Tiikkaja <marko(at)joh(dot)to> |
|---|---|
| To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Silly coding in pgcrypto |
| Date: | 2014-11-02 04:10:25 |
| Message-ID: | 5455AEB1.7030801@joh.to |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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 | Content-Type | Size |
|---|---|---|
| pgcrypto_sillyness.patch | text/plain | 1.0 KB |
| From: | Noah Misch <noah(at)leadboat(dot)com> |
|---|---|
| To: | Marko Tiikkaja <marko(at)joh(dot)to> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Silly coding in pgcrypto |
| Date: | 2014-11-02 21:34:34 |
| Message-ID: | 20141102213434.GA540883@tornado.leadboat.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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?
| From: | Tomas Vondra <tv(at)fuzzy(dot)cz> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Silly coding in pgcrypto |
| Date: | 2014-11-02 21:51:31 |
| Message-ID: | 5456A763.90206@fuzzy.cz |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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
| From: | Marko Tiikkaja <marko(at)joh(dot)to> |
|---|---|
| To: | Noah Misch <noah(at)leadboat(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Silly coding in pgcrypto |
| Date: | 2014-11-02 21:53:27 |
| Message-ID: | 5456A7D7.5010506@joh.to |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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
| From: | Noah Misch <noah(at)leadboat(dot)com> |
|---|---|
| To: | Marko Tiikkaja <marko(at)joh(dot)to> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Silly coding in pgcrypto |
| Date: | 2014-11-03 02:53:29 |
| Message-ID: | 20141103025329.GA537868@tornado.leadboat.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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.