From: Eric Biggers <ebiggers@kernel.org>
To: Yuzhuo Jing <yuzhuo@google.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
"David S . Miller" <davem@davemloft.net>,
Ian Rogers <irogers@google.com>,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 0/2] crypto: Fix sha1 compile error
Date: Fri, 13 Jun 2025 21:53:23 -0700 [thread overview]
Message-ID: <20250614045323.GF1284@sol> (raw)
In-Reply-To: <20250614000828.311722-1-yuzhuo@google.com>
On Fri, Jun 13, 2025 at 05:08:26PM -0700, Yuzhuo Jing wrote:
> This is a followup patch series for an ongoing patch series to reuse
> kernel tree sha1 utils in perf tools and remove libcrypto dependency.
> This mirrors the fixes made in perf back to the kernel tree so we can
> use tools/perf/check-headers.sh to monitor future changes.
> Link: https://lore.kernel.org/lkml/aC9lXhPFcs5fkHWH@x1/t/#u
>
> This series contains two patches: one fixing signed and unsigned integer
> comparisons and another fixing function type mismatches.
>
> Yuzhuo Jing (2):
> crypto: Fix sha1 signed integer comparison compile error
> crypto: Fix sha1 signed pointer comparison compile error
>
> crypto/sha1_generic.c | 2 +-
> include/crypto/sha1_base.h | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
I don't like these signedness inconsistencies in the code either, and I'll be
fixing these (among many other issues) when I refactor SHA-1 to have a proper
lib/crypto/ API similar to what I'm currently doing with SHA-2. That being
said, the kernel doesn't have these warnings enabled, and especially in its
current state this code isn't really designed to be copied into a userspace
program.
So I feel that the premise of this patchset, and more importantly also the one
you linked to above for tools/perf/, is a bit misguided.
I've sent an alternative patchset for you to consider:
https://lore.kernel.org/all/20250614044133.660848-1-ebiggers@kernel.org/. It
adds a minimal SHA-1 implementation, including a test, to tools/perf/util/. The
SHA-1 implementation is less than 100 lines anyway.
The effort it would take to "share" the kernel's code here is just not worth it,
IMO. Especially when I have some significant refactoring planned on the kernel
side which would make the tools/perf copy diverge anyway.
- Eric
prev parent reply other threads:[~2025-06-14 4:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-14 0:08 [PATCH v1 0/2] crypto: Fix sha1 compile error Yuzhuo Jing
2025-06-14 0:08 ` [PATCH v1 1/2] crypto: Fix sha1 signed integer comparison " Yuzhuo Jing
2025-06-14 0:08 ` [PATCH v1 2/2] crypto: Fix sha1 signed pointer " Yuzhuo Jing
2025-06-14 4:53 ` Eric Biggers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250614045323.GF1284@sol \
--to=ebiggers@kernel.org \
--cc=davem@davemloft.net \
--cc=herbert@gondor.apana.org.au \
--cc=irogers@google.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=yuzhuo@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.