Re: [PATCH v3 0/4] Use AF_ALG in checksum utilities

[Bruno Haible]

This patch addresses the code duplication issue in the module descriptions.
At the same time, it enables some of the best practices for gnulib modules.
In particular, the best practice is not to write

  #if some_condition
  #include "af_alg.h"
  #endif

but instead

  #include "af_alg.h"

and make sure that af_alg.h defines trivial replacement stubs on platforms
that don't implement the functionality.


2018-05-05  Bruno Haible  <address@hidden>

        af_alg: New module.
        * lib/af_alg.h: Test HAVE_* macro through '#if', not '#ifdef'.
        * lib/af_alg.c: Include "af_alg.h" before the other header files.
        * lib/md5.c: Include "af_alg.h" unconditionally.
        (md5_stream): Invoke afalg_stream unconditionally.
        * lib/sha1.c: Include "af_alg.h" unconditionally.
        (sha1_stream): Invoke afalg_stream unconditionally.
        * lib/sha256.c: Include "af_alg.h" unconditionally.
        (sha256_stream, sha224_stream): Invoke afalg_stream unconditionally.
        * lib/sha512.c: Include "af_alg.h" unconditionally.
        (sha512_stream, sha384_stream): Invoke afalg_stream unconditionally.
        * m4/af_alg.m4: Renamed from m4/linux-if-alg.m4.
        (gl_AF_ALG): Renamed from gl_LINUX_IF_ALG_H.
        * modules/crypto/af_alg: New file.
        * modules/crypto/md5 (Files): Remove files that are now in the
        'crypto/af_alg' module.
        (Depends-on): Add crypto/af_alg.
        (configure.ac): Remove gl_LINUX_IF_ALG_H invocation.
        (Makefile.am): Don't mention af_alg.c here.
        * modules/crypto/sha1 (Files): Remove files that are now in the
        'crypto/af_alg' module.
        (Depends-on): Add crypto/af_alg.
        (configure.ac): Remove gl_LINUX_IF_ALG_H invocation.
        (Makefile.am): Don't mention af_alg.c here.
        * modules/crypto/sha256 (Files): Remove files that are now in the
        'crypto/af_alg' module.
        (Depends-on): Add crypto/af_alg.
        (configure.ac): Remove gl_LINUX_IF_ALG_H invocation.
        (Makefile.am): Don't mention af_alg.c here.
        * modules/crypto/sha512 (Files): Remove files that are now in the
        'crypto/af_alg' module.
        (Depends-on): Add crypto/af_alg.
        (configure.ac): Remove gl_LINUX_IF_ALG_H invocation.
        (Makefile.am): Don't mention af_alg.c here.

diff --git a/lib/af_alg.c b/lib/af_alg.c
index 06344b1..73aa6e7 100644
--- a/lib/af_alg.c
+++ b/lib/af_alg.c
@@ -19,7 +19,9 @@
 
 #include <config.h>
 
-#ifdef HAVE_LINUX_IF_ALG_H
+#if HAVE_LINUX_IF_ALG_H
+
+#include "af_alg.h"
 
 #include <unistd.h>
 #include <string.h>
@@ -28,8 +30,6 @@
 #include <sys/sendfile.h>
 #include <sys/socket.h>
 
-#include "af_alg.h"
-
 #include "sys-limits.h"
 
 #define BLOCKSIZE 32768
diff --git a/lib/af_alg.h b/lib/af_alg.h
index aacb8f6..295f0f9 100644
--- a/lib/af_alg.h
+++ b/lib/af_alg.h
@@ -36,7 +36,7 @@
 extern "C" {
 # endif
 
-# ifdef HAVE_LINUX_IF_ALG_H
+# if HAVE_LINUX_IF_ALG_H
 
 /* Computes a message digest of the contents of a file.
    STREAM is an open file stream.  Regular files are handled more efficiently
diff --git a/lib/md5.c b/lib/md5.c
index e6bd209..2bf2f0c 100644
--- a/lib/md5.c
+++ b/lib/md5.c
@@ -32,9 +32,7 @@
 #include <string.h>
 #include <sys/types.h>
 
-#ifdef HAVE_LINUX_IF_ALG_H
-# include "af_alg.h"
-#endif
+#include "af_alg.h"
 
 #if USE_UNLOCKED_IO
 # include "unlocked-io.h"
@@ -148,16 +146,14 @@ md5_stream (FILE *stream, void *resblock)
   size_t sum;
   char *buffer;
 
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
-
-  ret = afalg_stream (stream, "md5", resblock, MD5_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream (stream, "md5", resblock, MD5_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
diff --git a/lib/sha1.c b/lib/sha1.c
index b670267..e7cd2a3 100644
--- a/lib/sha1.c
+++ b/lib/sha1.c
@@ -33,9 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#ifdef HAVE_LINUX_IF_ALG_H
-# include "af_alg.h"
-#endif
+#include "af_alg.h"
 
 #if USE_UNLOCKED_IO
 # include "unlocked-io.h"
@@ -127,7 +125,7 @@ sha1_finish_ctx (struct sha1_ctx *ctx, void *resbuf)
 #endif
 
 /* Compute SHA1 message digest for bytes read from STREAM.  The
-   resulting message digest number will be written into the 16 bytes
+   resulting message digest number will be written into the 20 bytes
    beginning at RESBLOCK.  */
 int
 sha1_stream (FILE *stream, void *resblock)
@@ -135,16 +133,15 @@ sha1_stream (FILE *stream, void *resblock)
   struct sha1_ctx ctx;
   size_t sum;
   char *buffer;
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
 
-  ret = afalg_stream (stream, "sha1", resblock, SHA1_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream (stream, "sha1", resblock, SHA1_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
diff --git a/lib/sha256.c b/lib/sha256.c
index fab119a..410bd98 100644
--- a/lib/sha256.c
+++ b/lib/sha256.c
@@ -32,9 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#ifdef HAVE_LINUX_IF_ALG_H
-# include "af_alg.h"
-#endif
+#include "af_alg.h"
 
 #if USE_UNLOCKED_IO
 # include "unlocked-io.h"
@@ -183,16 +181,14 @@ sha256_stream (FILE *stream, void *resblock)
   size_t sum;
   char *buffer;
 
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
-
-  ret = afalg_stream(stream, "sha256", resblock, SHA256_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream (stream, "sha256", resblock, SHA256_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
@@ -266,16 +262,14 @@ sha224_stream (FILE *stream, void *resblock)
   size_t sum;
   char *buffer;
 
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
-
-  ret = afalg_stream (stream, "sha224", resblock, SHA224_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream(stream, "sha224", resblock, SHA224_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
diff --git a/lib/sha512.c b/lib/sha512.c
index ff7c0cb..7a02348 100644
--- a/lib/sha512.c
+++ b/lib/sha512.c
@@ -32,9 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-#ifdef HAVE_LINUX_IF_ALG_H
-# include "af_alg.h"
-#endif
+#include "af_alg.h"
 
 #if USE_UNLOCKED_IO
 # include "unlocked-io.h"
@@ -191,16 +189,14 @@ sha512_stream (FILE *stream, void *resblock)
   size_t sum;
   char *buffer;
 
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
-
-  ret = afalg_stream (stream, "sha512", resblock, SHA512_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream (stream, "sha512", resblock, SHA512_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
@@ -274,16 +270,14 @@ sha384_stream (FILE *stream, void *resblock)
   size_t sum;
   char *buffer;
 
-#ifdef HAVE_LINUX_IF_ALG_H
-  int ret;
-
-  ret = afalg_stream (stream, "sha384", resblock, SHA384_DIGEST_SIZE);
-  if (!ret)
+  {
+    int ret = afalg_stream(stream, "sha384", resblock, SHA384_DIGEST_SIZE);
+    if (!ret)
       return 0;
 
-  if (ret == -EIO)
+    if (ret == -EIO)
       return 1;
-#endif
+  }
 
   buffer = malloc (BLOCKSIZE + 72);
   if (!buffer)
diff --git a/m4/linux-if-alg.m4 b/m4/af_alg.m4
similarity index 90%
rename from m4/linux-if-alg.m4
rename to m4/af_alg.m4
index 3ed18ae..1c57e2c 100644
--- a/m4/linux-if-alg.m4
+++ b/m4/af_alg.m4
@@ -1,3 +1,4 @@
+# af_alg.m4 serial 1
 dnl Check whether linux/if_alg.h has needed features.
 
 dnl Copyright 2018 Free Software Foundation, Inc.
@@ -7,7 +8,7 @@ dnl with or without modifications, as long as this notice is 
preserved.
 
 dnl From Matteo Croce.
 
-AC_DEFUN_ONCE([gl_LINUX_IF_ALG_H],
+AC_DEFUN_ONCE([gl_AF_ALG],
 [
   AC_REQUIRE([gl_HEADER_SYS_SOCKET])
   AC_CACHE_CHECK([whether linux/if_alg.h has struct sockaddr_alg.],
@@ -24,6 +25,6 @@ AC_DEFUN_ONCE([gl_LINUX_IF_ALG_H],
        [gl_cv_header_linux_if_alg_salg=no])])
   if test "$gl_cv_header_linux_if_alg_salg" = yes; then
     AC_DEFINE([HAVE_LINUX_IF_ALG_H], [1],
-      [Define to 1 if you have `struct sockaddr_alg' defined.])
+      [Define to 1 if you have 'struct sockaddr_alg' defined.])
   fi
 ])
diff --git a/modules/crypto/af_alg b/modules/crypto/af_alg
new file mode 100644
index 0000000..c9602dc
--- /dev/null
+++ b/modules/crypto/af_alg
@@ -0,0 +1,27 @@
+Description:
+Compute message digest using kernel-supported cryptography algorithms.
+
+Files:
+lib/af_alg.h
+lib/af_alg.c
+lib/sys-limits.h
+m4/af_alg.m4
+
+Depends-on:
+sys_socket
+sys_stat
+
+configure.ac:
+gl_AF_ALG
+
+Makefile.am:
+lib_SOURCES += af_alg.c
+
+Include:
+"af_alg.h"
+
+License:
+LGPLv2+
+
+Maintainer:
+all
diff --git a/modules/crypto/md5 b/modules/crypto/md5
index 80f14c2..1d5130b 100644
--- a/modules/crypto/md5
+++ b/modules/crypto/md5
@@ -5,14 +5,11 @@ Files:
 lib/gl_openssl.h
 lib/md5.h
 lib/md5.c
-lib/af_alg.h
-lib/af_alg.c
-lib/sys-limits.h
 m4/gl-openssl.m4
 m4/md5.m4
-m4/linux-if-alg.m4
 
 Depends-on:
+crypto/af_alg
 extern-inline
 stdalign
 stdint
@@ -21,10 +18,9 @@ sys_stat
 
 configure.ac:
 gl_MD5
-gl_LINUX_IF_ALG_H
 
 Makefile.am:
-lib_SOURCES += md5.c af_alg.c
+lib_SOURCES += md5.c
 
 Include:
 "md5.h"
diff --git a/modules/crypto/sha1 b/modules/crypto/sha1
index 3bdf0ea..90bd7e4 100644
--- a/modules/crypto/sha1
+++ b/modules/crypto/sha1
@@ -5,14 +5,11 @@ Files:
 lib/gl_openssl.h
 lib/sha1.h
 lib/sha1.c
-lib/af_alg.h
-lib/af_alg.c
-lib/sys-limits.h
 m4/gl-openssl.m4
 m4/sha1.m4
-m4/linux-if-alg.m4
 
 Depends-on:
+crypto/af_alg
 extern-inline
 stdalign
 stdint
@@ -21,10 +18,9 @@ sys_stat
 
 configure.ac:
 gl_SHA1
-gl_LINUX_IF_ALG_H
 
 Makefile.am:
-lib_SOURCES += sha1.c af_alg.c
+lib_SOURCES += sha1.c
 
 Include:
 "sha1.h"
diff --git a/modules/crypto/sha256 b/modules/crypto/sha256
index d7c3735..633bf04 100644
--- a/modules/crypto/sha256
+++ b/modules/crypto/sha256
@@ -5,14 +5,11 @@ Files:
 lib/gl_openssl.h
 lib/sha256.h
 lib/sha256.c
-lib/af_alg.h
-lib/af_alg.c
-lib/sys-limits.h
 m4/gl-openssl.m4
 m4/sha256.m4
-m4/linux-if-alg.m4
 
 Depends-on:
+crypto/af_alg
 extern-inline
 stdalign
 stdint
@@ -21,10 +18,9 @@ sys_stat
 
 configure.ac:
 gl_SHA256
-gl_LINUX_IF_ALG_H
 
 Makefile.am:
-lib_SOURCES += sha256.c af_alg.c
+lib_SOURCES += sha256.c
 
 Include:
 "sha256.h"
diff --git a/modules/crypto/sha512 b/modules/crypto/sha512
index d6f9b99..cedc31c 100644
--- a/modules/crypto/sha512
+++ b/modules/crypto/sha512
@@ -5,14 +5,11 @@ Files:
 lib/gl_openssl.h
 lib/sha512.h
 lib/sha512.c
-lib/af_alg.h
-lib/af_alg.c
-lib/sys-limits.h
 m4/gl-openssl.m4
 m4/sha512.m4
-m4/linux-if-alg.m4
 
 Depends-on:
+crypto/af_alg
 extern-inline
 stdalign
 stdint
@@ -22,10 +19,9 @@ u64
 
 configure.ac:
 gl_SHA512
-gl_LINUX_IF_ALG_H
 
 Makefile.am:
-lib_SOURCES += sha512.c af_alg.c
+lib_SOURCES += sha512.c
 
 Include:
 "sha512.h"