> On 4 Dec 2020, at 08:05, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Nov 10, 2020 at 01:28:09PM +0900, Michael Paquier wrote:
>> The CF bot has been complaining on Windows and this issue is fixed in
>> the attached. A refresh of src/tools/msvc for pgcrypto was just
>> missing.
>
> Now that HEAD has the necessary infrastructure to be able to plug in
> easily new cryptohash routines, here is a rebased patch for MD5.
This is a pretty straightforward patch which given the cryptohash framework
added previously does what it says on the tin. +1 on rolling MD5 into what we
already have for SHA2 now. Applies clean and all tests pass.
One (two-part) comment on the patch though:
The re-arrangement of the code does lead to some attribution confusion however
IMO. pgcrypto/md5.c and src/common/md5.c are combined with the actual MD5
implementation from pgcrypto/md5.c and the PG glue code from src/common/md5.c.
That is in itself a good choice, but the file headers are intact and now claims
two implementations of which only one remains.
Further, bytesToHex was imported in commit 957613be18e6b7 with attribution to
"Sverre H. Huseby <sverrehu@online.no>" without a PGDG defined copyright,
which was later added in 2ca65f716aee9ec441dda91d91b88dd7a00bffa1. This patch
moves bytesToHex from md5.c (where his attribution is) to md5_common.c with no
maintained attribution. Off the cuff it seems we should either attribute in a
comment, or leave the function and export it, but the usual "I'm not a lawyer"
disclaimer applies. Do you have any thoughts?
> The amount of code shaved is still nice:
> 13 files changed, 641 insertions(+), 775 deletions(-)
Always nice with a net minus patch with sustained functionality.
cheers ./daniel