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