于 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 >
