Re: [PATCH 3/5] lib/crc32: standardize on crc32c() name for Castagnoli CRC32

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Feb 07, 2025 at 03:42:33PM -0700, Nathan Chancellor wrote:
> Hi Eric,
> 
> On Tue, Feb 04, 2025 at 04:54:01PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > For historical reasons, the Castagnoli CRC32 is available under 3 names:
> > crc32c(), crc32c_le(), __crc32c_le().  Most callers use crc32c().  The
> > more verbose versions are not really warranted; there is no "_be"
> > version that the "_le" version needs to be differentiated from, and the
> > leading underscores are pointless.
> > 
> > Therefore, let's standardize on just crc32c().  Remove the other two
> > names, and update callers accordingly.
> > 
> > Specifically, the new crc32c() comes from what was previously
> > __crc32c_le(), so compared to the old crc32c() it now takes a size_t
> > length rather than unsigned int, and it's now in linux/crc32.h instead
> > of just linux/crc32c.h (which includes linux/crc32.h).
> > 
> > Later patches will also rename __crc32c_le_combine(), crc32c_le_base(),
> > and crc32c_le_arch().
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> ...
> > diff --git a/include/linux/crc32.h b/include/linux/crc32.h
> > index 61a7ec29d6338..bc39b023eac0f 100644
> > --- a/include/linux/crc32.h
> > +++ b/include/linux/crc32.h
> > @@ -27,12 +27,11 @@ static inline u32 crc32_be(u32 crc, const void *p, size_t len)
> >  	if (IS_ENABLED(CONFIG_CRC32_ARCH))
> >  		return crc32_be_arch(crc, p, len);
> >  	return crc32_be_base(crc, p, len);
> >  }
> >  
> > -/* TODO: leading underscores should be dropped once callers have been updated */
> > -static inline u32 __crc32c_le(u32 crc, const void *p, size_t len)
> > +static inline u32 crc32c(u32 crc, const void *p, size_t len)
> >  {
> >  	if (IS_ENABLED(CONFIG_CRC32_ARCH))
> >  		return crc32c_le_arch(crc, p, len);
> >  	return crc32c_le_base(crc, p, len);
> >  }
> 
> I think this rename is responsible for a build failure I see with an
> ARCH=mips configuration on current -next:
> 
>   $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- mrproper 32r6_defconfig arch/mips/lib/crc32-mips.o
>   arch/mips/lib/crc32-mips.c:25:9: error: 'crc32c' redeclared as different kind of symbol
>      25 |         crc32c,
>         |         ^~~~~~
>   In file included from arch/mips/lib/crc32-mips.c:12:
>   include/linux/crc32.h:32:19: note: previous definition of 'crc32c' with type 'u32(u32,  const void *, size_t)' {aka 'unsigned int(unsigned int,  const void *, unsigned int)'}
>      32 | static inline u32 crc32c(u32 crc, const void *p, size_t len)
>         |                   ^~~~~~
> 

Thanks for pointing this out!  I temporarily dropped the series from crc-next
and will send out a revised version soon.  The enum value in crc32-mips.c will
need to be renamed.

- Eric