On 8/22/14, 2:57 AM, Thomas Munro wrote:
I took a quick look at your patch at
http://www.postgresql.org/message-id/53edbcf0.9070...@joh.to (sorry I
didn't reply directly as I didn't have the message).  It applies
cleanly, builds, and the tests pass.  I will hopefully have more to
say after I've poked at it and understood it better, but in the
meantime a couple of trivial things I spotted:


Thanks!

In pgp-pgsql.c, function pgp_sym_decrypt_verify_bytea:

I think the first 'arg' should be 'psw'.

I think the same mistake appears in pgp_sym_decrypt_verify_text.

Yeah, these look like copypaste-os.  Will fix.

Should t be of type pg_time_t rather than time_t?  Would it be better
if PGP_Signature's member creation_time were of type uint32_t and you
could use ntohl(sig->creation_time) instead of the bitshifting?

I tried to make the code look like the existing code in init_litdata_packet(). I don't oppose to writing it this way, but I think we should change both instances if so (or perhaps move the code into a new function).

In pgp.h:

+#define PGP_MAX_KEY                                    (256/8)
+#define PGP_MAX_BLOCK                          (256/8)
+#define PGP_MAX_DIGEST                         (512/8)
+#define PGP_MAX_DIGEST_ASN1_PREFIX     20
+#define PGP_S2K_SALT                           8

The PGP_MAX_DIGEST_ASN1_PREFIX line is new but the others just have
whitespace changes, and I gather the done thing is not to reformat
existing lines like that to distract from the changes in a patch.

Right.  Sorry about that.  I can revert the whitespace fixes.

(Just curious, why do you use while (1) for an infinite loop in
extract_signatures, but for (;;) in pullf_discard?  It looks like the
latter is much more common in the source tree.)

In the postgres source tree? Yeah. But while(1) is all over pgcrypto, so I've tried to make the new code match that. If there are any instances of "for (;;)" in the patch, those must be because of me typing without thinking. I could search-and-replace those to "while (1)" to make it consistent.


.marko


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

Reply via email to