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

linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed

* [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice
@ 2025-11-02  5:36 kernel test robot
  2025-11-02 13:42 ` Kuan-Wei Chiu
  0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2025-11-02  5:36 UTC (permalink / raw)
  To: Kuan-Wei Chiu
  Cc: oe-kbuild-all, Andrew Morton, Linux Memory Management List, Guan-Chun Wu

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

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice
  2025-11-02  5:36 [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice kernel test robot
@ 2025-11-02 13:42 ` Kuan-Wei Chiu
  2025-11-02 14:42   ` David Laight
  0 siblings, 1 reply; 4+ messages in thread
From: Kuan-Wei Chiu @ 2025-11-02 13:42 UTC (permalink / raw)
  To: kernel test robot
  Cc: oe-kbuild-all, Andrew Morton, Linux Memory Management List,
	Guan-Chun Wu, David Laight

+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?

[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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice
  2025-11-02 13:42 ` Kuan-Wei Chiu
@ 2025-11-02 14:42   ` David Laight
  2025-11-02 14:57     ` Kuan-Wei Chiu
  0 siblings, 1 reply; 4+ messages in thread
From: David Laight @ 2025-11-02 14:42 UTC (permalink / raw)
  To: Kuan-Wei Chiu
  Cc: kernel test robot, oe-kbuild-all, Andrew Morton,
	Linux Memory Management List, Guan-Chun Wu

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.

	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  



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice
  2025-11-02 14:42   ` David Laight
@ 2025-11-02 14:57     ` Kuan-Wei Chiu
  0 siblings, 0 replies; 4+ messages in thread
From: Kuan-Wei Chiu @ 2025-11-02 14:57 UTC (permalink / raw)
  To: David Laight, Luc Van Oostenryck
  Cc: kernel test robot, oe-kbuild-all, Andrew Morton,
	Linux Memory Management List, Guan-Chun Wu

+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  
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-11-02 14:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-02  5:36 [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice kernel test robot
2025-11-02 13:42 ` Kuan-Wei Chiu
2025-11-02 14:42   ` David Laight
2025-11-02 14:57     ` Kuan-Wei Chiu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox