Hi Bruno,

Bruno Haible <br...@clisp.org> writes:

>> In a testdir of all crypto modules:
>> 
>>       CC       gc-gnulib.o
>
> Please take the habit to show the compiler command line, not only the
> summary line. It's important in cases like this because ...

Ah, I had assumed that the warning message would always have the proper
switch:

    gc-gnulib.c:351:48: warning: format ‘%x’ expects argument of type
    ‘unsigned int’, but argument 3 has type ‘int’ [-Wformat=]

So I didn't think sharing the full output was needed. But since you have
shown this isn't the case for -Wformat (and likely some others), I will
remember to include it next time.

> It's also possible to silence the warning without adding a cast:
>
>           sprintf (&keyMaterial[2 * i], "%02x", key[i] & 0xFFU);
>
> Casts can do so many unwanted things (such as converting from a pointer type
> or truncating some bits) that here I would prefer a code without a cast.

I agree, it is best to avoid casts. I pushed the attached patch with
your suggestion.

Collin

>From a90d0ae6c84f3496d247d251194fc3eab150304e Mon Sep 17 00:00:00 2001
Message-ID: <a90d0ae6c84f3496d247d251194fc3eab150304e.1748289421.git.collin.fu...@gmail.com>
From: Collin Funk <collin.fu...@gmail.com>
Date: Mon, 26 May 2025 12:56:22 -0700
Subject: [PATCH] crypto/gc: Simplify the previous change.

Suggested by Bruno Haible in:
<https://lists.gnu.org/archive/html/bug-gnulib/2025-05/msg00249.html>.

* lib/gc-gnulib.c (gc_cipher_setkey, gc_cipher_setiv): Remove cast and
perform a bitwise AND with an unsigned constant.
---
 ChangeLog       | 6 ++++++
 lib/gc-gnulib.c | 6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a6728386ac..9351692a28 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2025-05-26  Collin Funk  <collin.fu...@gmail.com>
 
+	crypto/gc: Simplify the previous change.
+	Suggested by Bruno Haible in:
+	<https://lists.gnu.org/archive/html/bug-gnulib/2025-05/msg00249.html>.
+	* lib/gc-gnulib.c (gc_cipher_setkey, gc_cipher_setiv): Remove cast and
+	perform a bitwise AND with an unsigned constant.
+
 	crypto/gc: Pacify -Wformat warnings.
 	* lib/gc-gnulib.c (gc_cipher_setkey, gc_cipher_setiv): Cast the argument
 	since "%02x" expects the argument to be unsigned.
diff --git a/lib/gc-gnulib.c b/lib/gc-gnulib.c
index 85b7a62b74..9b53feb886 100644
--- a/lib/gc-gnulib.c
+++ b/lib/gc-gnulib.c
@@ -290,8 +290,7 @@ gc_cipher_setkey (gc_cipher_handle handle, size_t keylen, const char *key)
         char keyMaterial[RIJNDAEL_MAX_KEY_SIZE + 1];
 
         for (i = 0; i < keylen; i++)
-          sprintf (&keyMaterial[2 * i], "%02x",
-                   (unsigned int) (key[i] & 0xFF));
+          sprintf (&keyMaterial[2 * i], "%02x", key[i] & 0xFFU);
 
         rc = rijndaelMakeKey (&ctx->aesEncKey, RIJNDAEL_DIR_ENCRYPT,
                               keylen * 8, keyMaterial);
@@ -349,8 +348,7 @@ gc_cipher_setiv (gc_cipher_handle handle, size_t ivlen, const char *iv)
             char ivMaterial[2 * RIJNDAEL_MAX_IV_SIZE + 1];
 
             for (i = 0; i < ivlen; i++)
-              sprintf (&ivMaterial[2 * i], "%02x",
-                       (unsigned int) (iv[i] & 0xFF));
+              sprintf (&ivMaterial[2 * i], "%02x", iv[i] & 0xFFU);
 
             rc = rijndaelCipherInit (&ctx->aesContext, RIJNDAEL_MODE_CBC,
                                      ivMaterial);
-- 
2.49.0

Reply via email to