Re: [PATCH v1 0/2] crypto: Fix sha1 compile error

Eric Biggers
2025-06-14T04:53:51Z

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