[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
