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

[Eric Biggers]

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@xxxxxxxxxx/.  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