Subject: Re: [PATCH 09/61] bootwrapper: Add dt_is_compatible().

From: Scott Wood <[email-masked]>
Date: Wed, 18 Jul 2007 13:24:13 -0500

Milton Miller wrote:
>> +       for (pos = 0; pos + compat_len < len; pos++) {
>> +               if (!strcmp(buf + pos, compat))
>> +                       return 1;
>> +
>> +               while (buf[pos] && pos + compat_len < len)
>> +                       pos++;
> 
> 
> This is buggy: if you are searching for "ns16550" and the compatable is 
> "fsl,1234\0commons16550" this code will incorrectly says its compatable.

How so?

The first iteration will compare "ns16550" to "fsl,1234", find that they 
don't match, and advance to the '\0'.

The second iteration will compare "ns16550" to "commons16550", find that 
they don't match, and advance to the 'n'.

The third iteration will not happen, since the loop condition fails, and 
not-compatible is returned.

> Comparing pos < len instead will do the right thing, at the cost of a 
> few iterations of the loop.

I'll grant that the extra iterations aren't worth doing the strlen() -- 
I'm not sure why I did it that way.

-Scott