Discussion:
[V3 PATCH] Implement SM3 hash algorithm in gnulib
Jia Zhang
2017-10-28 14:27:10 UTC
Permalink
Hi Bruno,

Here is the V3 changelog:

- Correct @LIB_CRYPTO@ build issue in m4/sm3.m4.
- Merge patch 4 and 5 together.
- Make the commit header more readable for patch 2,3,4, and add the
short file change descriptions.

Thanks,
Jia
Bruno Haible
2017-10-28 20:25:12 UTC
Permalink
Hi Jia,
Thanks for the update. The module 'crypto/sm3' passes the test, so I pushed
that part of the patch in your name.

The module 'crypto/gc-sm3' does not pass the test:

$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure crypto/gc-sm3
$ cd testdir1
$ ./configure CPPFLAGS=-Wall
$ make
...
make[4]: Entering directory '/media/develdata/devel/GNULIB/gnulib-git/testdir1/gltests'
gcc -DHAVE_CONFIG_H -I. -I.. -DGNULIB_STRICT_CHECKING=1 -DIN_GNULIB_TESTS=1 -I. -I. -I.. -I./.. -I../gllib -I./../gllib -Wall -g -O2 -MT test-gc-sm3.o -MD -MP -MF .deps/test-gc-sm3.Tpo -c -o test-gc-sm3.o test-gc-sm3.c
test-gc-sm3.c:22:20: fatal error: gcrypt.h: No such file or directory
compilation terminated.
Makefile:1009: recipe for target 'test-gc-sm3.o' failed

The problem is most likely in the 'Files' or 'Dependencies' section of one
of the two modules. You find the reference doc for the modules descriptions
here:
https://www.gnu.org/software/gnulib/manual/html_node/Module-description.html


Also, I added a comment with the useful info that you gave in the commit message:

2017-10-28 Bruno Haible <***@clisp.org>

crypto/sm3: Add overview documentation to the .h file.
* lib/sm3.h: Add comments.

Rationale: Future maintainers should be able to understand and maintain this
code without looking at the commit history or ChangeLog.
Post by Jia Zhang
- Merge patch 4 and 5 together.
There's a misunderstanding here: I did not mean to declare all functions 'const',
but only the 3 you mentioned: gc_init, gc_done, gc_hash_digest_length.

And looking at the definition of the 'const' semantics [1], I was wrong on 2 of
these: If gc-gnulib.c is in use,
* 'gc_init' cannot be declared 'const' because
gc_init(); gc_done(); gc_init();
is not equivalent to
gc_init(); gc_done();
* 'gc_done' cannot be declared 'const' because
gc_init(); gc_done(); gc_init(); gc_done();
is not equivalent to
gc_init(); gc_done(); gc_init();
Only 'gc_hash_digest_length' may be declared 'const'.

Bruno

[1] https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Common-Function-Attributes.html
Jia Zhang
2017-10-29 01:54:36 UTC
Permalink
Post by Bruno Haible
Hi Jia,
Thanks for the update. The module 'crypto/sm3' passes the test, so
I pushed that part of the patch in your name.
$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure
crypto/gc-sm3 $ cd testdir1 $ ./configure CPPFLAGS=-Wall $ make ...
make[4]: Entering directory
'/media/develdata/devel/GNULIB/gnulib-git/testdir1/gltests' gcc
-DHAVE_CONFIG_H -I. -I.. -DGNULIB_STRICT_CHECKING=1
-DIN_GNULIB_TESTS=1 -I. -I. -I.. -I./.. -I../gllib -I./../gllib
-Wall -g -O2 -MT test-gc-sm3.o -MD -MP -MF .deps/test-gc-sm3.Tpo
-c -o test-gc-sm3.o test-gc-sm3.c test-gc-sm3.c:22:20: fatal
error: gcrypt.h: No such file or directory compilation terminated.
Makefile:1009: recipe for target 'test-gc-sm3.o' failed
The problem is most likely in the 'Files' or 'Dependencies'
section of one of the two modules. You find the reference doc for
https://www.gnu.org/software/gnulib/manual/html_node/Module-description.html
I don't have this issue. Maybe /usr/include/gcrypt.h doesn't exist in
your environment?

Succeeded to compile with gc-gnulib.c:
$ ./configure
$ grep HAVE_LIBGCRYPT config.h
/* #undef HAVE_LIBGCRYPT */

Succeeded to compile with gc-libgcrypt.c:
$ ./configure --with-libgcrypt
$ grep HAVE_LIBGCRYPT config.h
#define HAVE_LIBGCRYPT 1

Also, could you try to compile crypto/gc and see whether it has the
same failure?
Post by Bruno Haible
crypto/sm3: Add overview documentation to the .h file. *
lib/sm3.h: Add comments.
Rationale: Future maintainers should be able to understand and
maintain this code without looking at the commit history or
ChangeLog.
Post by Jia Zhang
- Merge patch 4 and 5 together.
There's a misunderstanding here: I did not mean to declare all
functions 'const', but only the 3 you mentioned: gc_init, gc_done,
gc_hash_digest_length.
And looking at the definition of the 'const' semantics [1], I was
wrong on 2 of these: If gc-gnulib.c is in use, * 'gc_init' cannot
be declared 'const' because gc_init(); gc_done(); gc_init(); is
not equivalent to gc_init(); gc_done(); * 'gc_done' cannot be
declared 'const' because gc_init(); gc_done(); gc_init();
gc_done(); is not equivalent to gc_init(); gc_done(); gc_init();
Only 'gc_hash_digest_length' may be declared 'const'.
Here is the full list of const issues:

[gc-gnulib.c] 8 failures
lib/gc-gnulib.c: In function 'gc_init':
lib/gc-gnulib.c:87:1: error: function might be candidate for attribute
'const' [-Werror=suggest-attribute=const]
gc_init (void)
^~~~~~~
lib/gc-gnulib.c: In function 'gc_done':
lib/gc-gnulib.c:114:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_done (void)
^~~~~~~
lib/gc-gnulib.c: In function 'gc_set_allocators':
lib/gc-gnulib.c:217:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_set_allocators (gc_malloc_t func_malloc,
^~~~~~~~~~~~~~~~~
lib/gc-gnulib.c: In function 'gc_cipher_setkey':
lib/gc-gnulib.c:334:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_setkey (gc_cipher_handle handle, size_t keylen, const char
*key)
^~~~~~~~~~~~~~~~
lib/gc-gnulib.c: In function 'gc_cipher_setiv':
lib/gc-gnulib.c:398:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_setiv (gc_cipher_handle handle, size_t ivlen, const char *iv)
^~~~~~~~~~~~~~~
lib/gc-gnulib.c: In function 'gc_cipher_encrypt_inline':
lib/gc-gnulib.c:452:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_encrypt_inline (gc_cipher_handle handle, size_t len, char
*data)
^~~~~~~~~~~~~~~~~~~~~~~~
lib/gc-gnulib.c: In function 'gc_cipher_decrypt_inline':
lib/gc-gnulib.c:522:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_decrypt_inline (gc_cipher_handle handle, size_t len, char
*data)
^~~~~~~~~~~~~~~~~~~~~~~~
lib/gc-gnulib.c: In function 'gc_hash_digest_length':
lib/gc-gnulib.c:706:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_hash_digest_length (Gc_hash hash)
^~~~~~~~~~~~~~~~~~~~~

[gc-libgcrypt.c] 2 failures
lib/gc-libgcrypt.c: In function 'gc_done':
lib/gc-libgcrypt.c:66:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_done (void)
^~~~~~~
lib/gc-libgcrypt.c: In function 'gc_hash_digest_length':
lib/gc-libgcrypt.c:368:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_hash_digest_length (Gc_hash hash)
^~~~~~~~~~~~~~~~~~~~~

In order to get the consistent function definitions, I fixed all 8
functions in gc-gnulib.c and gc-libgcrypt.c and thus introduced 16
fixup. So in next PR I will only fix all 10 reported failures.

Jia
Post by Bruno Haible
Bruno
[1]
https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Common-Function-Attributes.html
Jia Zhang
2017-10-29 04:16:18 UTC
Permalink
Hi Bruno,

I sent a new submission based on the latest master branch for a clear
review without sm3 basis.

Plz refer to "[PATCH] sm3: support to compile with libgcrypt".

Let's continue the discussion over it.

Thanks,
Jia
Post by Jia Zhang
Post by Bruno Haible
Hi Jia,
Thanks for the update. The module 'crypto/sm3' passes the test, so
I pushed that part of the patch in your name.
$ ./gnulib-tool --create-testdir --dir=testdir1 --single-configure
crypto/gc-sm3 $ cd testdir1 $ ./configure CPPFLAGS=-Wall $ make ...
make[4]: Entering directory
'/media/develdata/devel/GNULIB/gnulib-git/testdir1/gltests' gcc
-DHAVE_CONFIG_H -I. -I.. -DGNULIB_STRICT_CHECKING=1
-DIN_GNULIB_TESTS=1 -I. -I. -I.. -I./.. -I../gllib -I./../gllib
-Wall -g -O2 -MT test-gc-sm3.o -MD -MP -MF .deps/test-gc-sm3.Tpo
-c -o test-gc-sm3.o test-gc-sm3.c test-gc-sm3.c:22:20: fatal
error: gcrypt.h: No such file or directory compilation terminated.
Makefile:1009: recipe for target 'test-gc-sm3.o' failed
The problem is most likely in the 'Files' or 'Dependencies'
section of one of the two modules. You find the reference doc for
https://www.gnu.org/software/gnulib/manual/html_node/Module-description.html
I don't have this issue. Maybe /usr/include/gcrypt.h doesn't exist in
your environment?
$ ./configure
$ grep HAVE_LIBGCRYPT config.h
/* #undef HAVE_LIBGCRYPT */
$ ./configure --with-libgcrypt
$ grep HAVE_LIBGCRYPT config.h
#define HAVE_LIBGCRYPT 1
Also, could you try to compile crypto/gc and see whether it has the
same failure?
Post by Bruno Haible
crypto/sm3: Add overview documentation to the .h file. *
lib/sm3.h: Add comments.
Rationale: Future maintainers should be able to understand and
maintain this code without looking at the commit history or
ChangeLog.
Post by Jia Zhang
- Merge patch 4 and 5 together.
There's a misunderstanding here: I did not mean to declare all
functions 'const', but only the 3 you mentioned: gc_init, gc_done,
gc_hash_digest_length.
And looking at the definition of the 'const' semantics [1], I was
wrong on 2 of these: If gc-gnulib.c is in use, * 'gc_init' cannot
be declared 'const' because gc_init(); gc_done(); gc_init(); is
not equivalent to gc_init(); gc_done(); * 'gc_done' cannot be
declared 'const' because gc_init(); gc_done(); gc_init();
gc_done(); is not equivalent to gc_init(); gc_done(); gc_init();
Only 'gc_hash_digest_length' may be declared 'const'.
[gc-gnulib.c] 8 failures
lib/gc-gnulib.c:87:1: error: function might be candidate for attribute
'const' [-Werror=suggest-attribute=const]
gc_init (void)
^~~~~~~
lib/gc-gnulib.c:114:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_done (void)
^~~~~~~
lib/gc-gnulib.c:217:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_set_allocators (gc_malloc_t func_malloc,
^~~~~~~~~~~~~~~~~
lib/gc-gnulib.c:334:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_setkey (gc_cipher_handle handle, size_t keylen, const char
*key)
^~~~~~~~~~~~~~~~
lib/gc-gnulib.c:398:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_setiv (gc_cipher_handle handle, size_t ivlen, const char *iv)
^~~~~~~~~~~~~~~
lib/gc-gnulib.c:452:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_encrypt_inline (gc_cipher_handle handle, size_t len, char
*data)
^~~~~~~~~~~~~~~~~~~~~~~~
lib/gc-gnulib.c:522:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_cipher_decrypt_inline (gc_cipher_handle handle, size_t len, char
*data)
^~~~~~~~~~~~~~~~~~~~~~~~
lib/gc-gnulib.c:706:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_hash_digest_length (Gc_hash hash)
^~~~~~~~~~~~~~~~~~~~~
[gc-libgcrypt.c] 2 failures
lib/gc-libgcrypt.c:66:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_done (void)
^~~~~~~
lib/gc-libgcrypt.c:368:1: error: function might be candidate for
attribute 'const' [-Werror=suggest-attribute=const]
gc_hash_digest_length (Gc_hash hash)
^~~~~~~~~~~~~~~~~~~~~
In order to get the consistent function definitions, I fixed all 8
functions in gc-gnulib.c and gc-libgcrypt.c and thus introduced 16
fixup. So in next PR I will only fix all 10 reported failures.
Jia
Post by Bruno Haible
Bruno
[1]
https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Common-Function-Attributes.html
Bruno Haible
2017-10-29 07:45:17 UTC
Permalink
Hi Jia,
Post by Jia Zhang
I don't have this issue. Maybe /usr/include/gcrypt.h doesn't exist in
your environment?
Correct, I don't have /usr/include/gcrypt.h in my environment.
Post by Jia Zhang
Also, could you try to compile crypto/gc and see whether it has the
same failure?
The testdir for module crypto/gc works fine in my environment.
$ grep LIBGCRYPT config.h
/* #undef HAVE_LIBGCRYPT */

Bruno

Loading...