Re: invalid tests on unsigned

[Mikael Pettersson]

roel kluin writes:
 > Using spatch I have found the following occurrences where there is
 > an invalid test on an unsigned.
 > 
 > Some may be not seriously, but just redundant, others may indicate
 > an incorrect assumption (that the variable can be negative).
 > I haven't found time to sort out these.
 > 
 > If I have time I will write patches, but feel free to write one
 > yourself if you want.
 > 
 > Roel
 > 
 > What is shown below is not meant as patch, just for reporting.
 > 
 > ----[ arch/arm/mach-davinci/psc.c, unsigned ]----
 > --- arch/arm/mach-davinci/psc.c	2008-07-19 23:16:48.000000000 +0200
 > @@ -70,7 +70,7 @@ void davinci_psc_config(unsigned int dom
 >  {
 >  	u32 epcpr, ptcmd, ptstat, pdstat, pdctl1, mdstat, mdctl, mdstat_mask;
 >  
 > -	if (id < 0)
 >  		return;
 >  
 >  	mdctl = davinci_readl(DAVINCI_PWR_SLEEP_CNTRL_BASE + MDCTL + 4 * id);

This and similar ones may be objectionable.

 > ----[ arch/sparc64/kernel/process.c, unsigned long ]----
 > --- arch/sparc64/kernel/process.c	2008-08-07 17:53:01.000000000 +0200
 > @@ -593,7 +593,7 @@ asmlinkage long sparc_do_fork(unsigned l
 >  	 * the parent's %o1.  So detect that case and restore it
 >  	 * here.
 >  	 */
 > -	if ((unsigned long)ret >= -ERESTART_RESTARTBLOCK)
 >  		regs->u_regs[UREG_I1] = orig_i1;
 >  
 >  	return ret;

This and similar ones are not wrong. The -constant is converted
to unsigned (which is a well-defined operation) after which an >=u
(greater-or-equal unsigned) is performed.

Also, when posting a patch that touches files in several areas
it's useful to prefix it with a 'diffstat', as that allows people
to quickly see if the patch touches something they're interested in.

/Mikael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/