Re: [PATCH 5/8] kselftest/arm64: mte: fix printf type warning about mask

[Andre Przywara <andre.przywara@arm.com>]

On Fri, 16 Aug 2024 17:30:14 +0100
Mark Brown <broonie@kernel.org> wrote:

Hi Mark,

thanks for having a look!

> On Fri, Aug 16, 2024 at 04:32:48PM +0100, Andre Przywara wrote:
> > When masking the return value of a prctl, which is clearly an "int", we
> > use a uapi header provided mask, which is defined using an "UL" modifier,
> > so the whole expression is promoted to a long. This upsets the compiler's
> > printf type checker, because we use "%x" in the format string.
> > 
> > While we could simply upgrade this to a "%lx", it sounds wrong to
> > promote the "ret" variable, that is clearly an int.
> > Downcast the mask instead, to keep the type correct.  
> 
> This suggests that we've got some confusion with the UAPI, these flags
> need to go through a prctl() return so they shouldn't be unsigned
> long...  That said, it's UAPI so I'm not sure that's fixable.

My thoughts, exactly. Not sure if this mask is used on some larger types
somewhere else, but it's indeed moot since it's UAPI.

> > -	if ((ret & PR_MTE_TCF_MASK) == mask) {
> > +	if ((ret & (int)PR_MTE_TCF_MASK) == mask) {
> >  		ksft_test_result_pass("%s\n", name);
> >  	} else {
> >  		ksft_print_msg("Got %x, expected %x\n",
> > -			       (ret & PR_MTE_TCF_MASK), mask);
> > +			       (ret & (int)PR_MTE_TCF_MASK), mask);
> >  		ksft_test_result_fail("%s\n", name);  
> 
> TBH my inclination is that this is worse than letting the value be
> promoted, casts (particularly casts of constants) are just obviously
> suspect in a way that printf() formats aren't.

Fair enough, I wasn't sure about this either, and indeed this down-cast of
a constant smells dodgy. So shall I just use %lx, and rely on the
promotion (so the MASK being defined as UL), or cast "ret" to long?

Cheers,
Andre