bug-gnulib
[Top][All Lists]
Advanced

[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: Bruno Haible
Subject: Re: [V2 PATCH] Implement SM3 hash algorithm in gnulib and the computing tool in coreutils.
Date: Sat, 28 Oct 2017 14:44:16 +0200
User-agent: KMail/5.1.3 (Linux/4.4.0-97-generic; KDE/5.18.0; x86_64; ; )

[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

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])

* 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?

Bruno




reply via email to

[Prev in Thread] Current Thread [Next in Thread]