On 9/26/16 2:02 AM, Michael Paquier wrote: > On Mon, Sep 26, 2016 at 2:15 AM, David Steele <da...@pgmasters.net> wrote: > > Thanks for the review and the comments! > >> I notice that the copyright from pgcrypto/sha1.c was carried over but >> not the copyright from pgcrypto/sha2.c. I'm no expert on how this >> works, but I believe the copyright from sha2.c must be copied over. > > Right, those copyright bits are missing: > - * AUTHOR: Aaron D. Gifford <m...@aarongifford.com> > [...] > - * Copyright (c) 2000-2001, Aaron D. Gifford > The license block being the same, it seems to me that there is no need > to copy it over. The copyright should be enough.
Looks fine to me. >> Also, are there any plans to expose these functions directly to the user >> without loading pgcrypto? Now that the functionality is in core it >> seems that would be useful. In addition, it would make this patch stand >> on its own rather than just being a building block. > > There have been discussions about avoiding enabling those functions by > default in the distribution. We'd rather not do that... OK. >> * [PATCH 2/8] Move encoding routines to src/common/ >> >> I wonder if it is confusing to have two of encode.h/encode.c. Perhaps >> they should be renamed to make them distinct? > > Yes it may be a good idea to rename that, like encode_utils.[c|h] for > the new files. I like that better. >> Couldn't md5_crypt_verify() be made more general and take the hash type? >> For instance, password_crypt_verify() with the last param as the new >> password type enum. > > This would mean incorporating the whole SASL message exchange into > this routine because the password string is part of the scram > initialization context, and it seems to me that it is better to just > do once a lookup at the entry in pg_authid. So we'd finish with a more > confusing code I am afraid. At least that's the conclusion I came up > with when doing that.. md5_crypt_verify does only the work on a > received password. Ah, yes, I see now. I missed that when I reviewed patch 6. -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers