[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [V2 PATCH] Implement SM3 hash algorithm in gnulib and the computing
From: |
Jia Zhang |
Subject: |
Re: [V2 PATCH] Implement SM3 hash algorithm in gnulib and the computing tool in coreutils. |
Date: |
Sat, 28 Oct 2017 21:01:32 +0800 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
于 2017/10/28 下午8:44, Bruno Haible 写道:
> [Dropping coreutils, since I'm replying only the gnulib part.]
>
> Hi Jia,
>
>> Plz refer to the following PRs for code review.
>>
>> - For the new gnulib module crypto/sm3
>> https://github.com/coreutils/gnulib/pull/3/commits
>
> Usually, on this mailing list, we share patches through 'git
> format-patch -1', not github. In order to view your patch, I had to
> 1. go to https://github.com/jiazhang0/gnulib/tree/sm3-devel 2. do a
> 'git clone' with the URL from there, 3. $ git checkout
> origin/sm3-devel 4. $ git format-patch -5
Sorry I will send V3 patch to here directly.
>
> Review comments: * "New module: crypto/sm3" looks very good. Just
> one thing is wrong: The HAVE_OPENSSL_SM3 check is not like in,
> say, m4/sha1.m4.
>
> You can see the problem by doing $ ./gnulib-tool --create-testdir
> --dir=testdir1 --single-configure crypto/sm3 $ cd testdir1 $
> ./configure CPPFLAGS=-Wall $ make ... gcc -g -O2 -o test-sm3
> test-sm3.o libtests.a ../gllib/libgnu.a libtests.a @LIB_CRYPTO@
> gcc: error: @LIB_CRYPTO@: No such file or directory Makefile:919:
> recipe for target 'test-sm3' failed
>
> How to fix this? If sm3 is already supported in openssl: use
> gl_CRYPTO_CHECK, like in sha1.m4. If not, initialize LIB_CRYPTO to
> empty, in a way that does not conflict with the other crypto
> modules: m4_divert_once([DEFAULTS], [LIB_CRYPTO=])
> AC_SUBST([LIB_CRYPTO])
Thanks for your comments. I fixed it and the test passes. I just feel
strange that test-sm3 was built out without error if I compiled gnulib
with coreutils together.
>
> * Patch 2 looks good, applied.
>
> * Patch 3 looks good, applied.
>
> * Patch 4, 5: GCC's warnings were correct (unlike GCC's warning
> for 'proper_name'). Instead of disabling the warning, could you
> mark the functions with _GL_ATTRIBUTE_CONST?
No problem. I will correct them.
Thanks,
Jia
>
> Bruno
>