linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Kuan-Wei Chiu <visitorckw@gmail.com>
Cc: kernel test robot <lkp@intel.com>,
	oe-kbuild-all@lists.linux.dev,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Guan-Chun Wu <409411716@gms.tku.edu.tw>
Subject: Re: [akpm-mm:mm-nonmm-unstable 87/91] lib/base64.c:36:28: sparse: sparse: Initializer entry defined twice
Date: Sun, 2 Nov 2025 14:42:51 +0000	[thread overview]
Message-ID: <20251102144251.7f0fa78c@pumpkin> (raw)
In-Reply-To: <aQdfzCq2JH5QX9AH@google.com>

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  



  reply	other threads:[~2025-11-02 14:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-02  5:36 kernel test robot
2025-11-02 13:42 ` Kuan-Wei Chiu
2025-11-02 14:42   ` David Laight [this message]
2025-11-02 14:57     ` Kuan-Wei Chiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251102144251.7f0fa78c@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=409411716@gms.tku.edu.tw \
    --cc=akpm@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    --cc=lkp@intel.com \
    --cc=oe-kbuild-all@lists.linux.dev \
    --cc=visitorckw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox