Re: [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice

[Kuan-Wei Chiu <visitorckw@gmail.com>]

+Cc Luc Van Oostenryck for sparse discussion

On Sun, Nov 02, 2025 at 02:42:51PM +0000, David Laight wrote:
> On Sun, 2 Nov 2025 21:42:36 +0800
> Kuan-Wei Chiu <visitorckw@gmail.com> wrote:
> 
> > +Cc David
> > 
> > On Sun, Nov 02, 2025 at 01:36:16PM +0800, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable
> > > head:   97751db460a7c6988b2ab988d9889d4309f9cc8c
> > > commit: 5b693a7ad2acfa88e8ab0a047335ea4c94fecdb1 [87/91] lib/base64: optimize base64_decode() with reverse lookup tables
> > > config: m68k-randconfig-r123-20251102 (https://download.01.org/0day-ci/archive/20251102/202511021343.107utehN-lkp@intel.com/config)
> > > compiler: m68k-linux-gcc (GCC) 14.3.0
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251102/202511021343.107utehN-lkp@intel.com/reproduce)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202511021343.107utehN-lkp@intel.com/
> > > 
> > > sparse warnings: (new ones prefixed by >>)  
> > > >> lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice  
> > >    lib/base64.c:36:28: sparse:   also defined here
> > >    lib/base64.c:37:25: sparse: sparse: Initializer entry defined twice
> > >    lib/base64.c:37:25: sparse:   also defined here
> > >   
> > 
> > I guess this warning is triggered because we first initialize the array
> > with [0 ... 255] = -1, and then re-assign values for ['A'], ['a'],
> > ['0'], as well as the 62nd and 63rd characters.
> > 
> > This approach was adopted based on David's suggestion [1] to improve
> > readability by avoiding the expansion of the large 256 * 3 table.
> > 
> > I'm uncertain whether we should reconsider this method to avoid the
> > warning, or if it's safe to ignore it in this case?
> 
> I was worried something might complain.
> But any other initialiser makes it pretty easy to forget to initialise
> one of the fields.
> I don't know whether the code can be annotated to stop sparse complaining,
> maybe sparse could be fixed to ignore a non-zero default initialiser.
> 
> The only other obvious initialiser is to pass in the values for "+-/_".
> eg:
> #define BASE64_REV_INIT(val_plus, val_comma, val_minus, val_slash, val_under) { \
> 	[ 0 ... '+'-1 ] = -1, \
> 	[ '+' ] = val_plus, val_comma, val_minus, -1, val_slash, \ 
> 	[ '0' ] = 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, \
> 	[ '9'+1 ... 'A'-1 ] = -1, \
> 	[ 'A' ] = 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, \
> 		  23, 24, 25, 26, 27, 28, 28, 30, 31, 32, 33, 34, 35, \
> 	[ 'Z'+1 ... '_'-1 ] = -1, \
> 	[ '_' ] = val_under, \
> 	[ '_'+1 ... 'a'-1 ] = -1, \
> 	[ 'a' ] = 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, \
> 		  49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, \
> 	[ 'z'+1 ... 255 ] = -1 \
> }
> 
> Then have:
> 	static const s8 base64_rev_maps[][256] = {
> 		[BASE64_STD] = BASE64_REV_INIT(62, -1, -1, 63, -1),  
> 		[BASE64_URLSAFE] = BASE64_REV_INIT(-1, -1, 62, -1, 63),  
> 		[BASE64_IMAP] = BASE64_REV_INIT(62, 63, -1, -1, -1)
> 	};
> 
> But even that is error-prone, and doesn't really scale.
> 
> The static testing tools shouldn't really make coding error-prone.
> So I'd definitely look for a way of stopping sparse complaining.
> 

I also think the current code is more readable and less error-prone.
I'll loop in Luc to see if he has any feedback or thoughts on this.

Regards,
Kuan-Wei

> 	David
> 
> 
> > 
> > [1]: https://lore.kernel.org/lkml/20250928195736.71bec9ae@pumpkin/
> > 
> > Regards,
> > Kuan-Wei
> > 
> > > vim +36 lib/base64.c
> > > 
> > >     33	
> > >     34	static const s8 base64_rev_maps[][256] = {
> > >     35		[BASE64_STD] = BASE64_REV_INIT('+', '/'),  
> > >   > 36		[BASE64_URLSAFE] = BASE64_REV_INIT('-', '_'),  
> > >     37		[BASE64_IMAP] = BASE64_REV_INIT('+', ',')
> > >     38	};
> > >     39	/**
> > >     40	 * base64_encode() - Base64-encode some binary data
> > >     41	 * @src: the binary data to encode
> > >     42	 * @srclen: the length of @src in bytes
> > >     43	 * @dst: (output) the Base64-encoded string.  Not NUL-terminated.
> > >     44	 * @padding: whether to append '=' padding characters
> > >     45	 * @variant: which base64 variant to use
> > >     46	 *
> > >     47	 * Encodes data using the selected Base64 variant.
> > >     48	 *
> > >     49	 * Return: the length of the resulting Base64-encoded string in bytes.
> > >     50	 */
> > >     51	int base64_encode(const u8 *src, int srclen, char *dst, bool padding, enum base64_variant variant)
> > >     52	{
> > >     53		u32 ac = 0;
> > >     54		int bits = 0;
> > >     55		int i;
> > >     56		char *cp = dst;
> > >     57		const char *base64_table = base64_tables[variant];
> > >     58	
> > >     59		for (i = 0; i < srclen; i++) {
> > >     60			ac = (ac << 8) | src[i];
> > >     61			bits += 8;
> > >     62			do {
> > >     63				bits -= 6;
> > >     64				*cp++ = base64_table[(ac >> bits) & 0x3f];
> > >     65			} while (bits >= 6);
> > >     66		}
> > >     67		if (bits) {
> > >     68			*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> > >     69			bits -= 6;
> > >     70		}
> > >     71		if (padding) {
> > >     72			while (bits < 0) {
> > >     73				*cp++ = '=';
> > >     74				bits += 2;
> > >     75			}
> > >     76		}
> > >     77		return cp - dst;
> > >     78	}
> > >     79	EXPORT_SYMBOL_GPL(base64_encode);
> > >     80	
> > > 
> > > -- 
> > > 0-DAY CI Kernel Test Service
> > > https://github.com/intel/lkp-tests/wiki  
>