Garrett D'Amore wrote:
>Sebastien Roy wrote:
>
>
>>Garrett D'Amore wrote:
>>
>>
>>>>* 2583-2589: I don't understand the need for the cascading chain of
>>>>if statements here. This isn't much of an improvement over the
>>>>previous code, and the indentation is still not cstyle compliant.
>>>>It could be simplified to:
>>>>
>>>> if ((GET_ROM8(&(hmep->hme_romp[i])) & 0xff) == 'P' &&
>>>> (GET_ROM8(&(hmep->hme_romp[i+1])) & 0xff) == 'C' &&
>>>> (GET_ROM8(&(hmep->hme_romp[i+2])) & 0xff) == 'I' &&
>>>> (GET_ROM8(&(hmep->hme_romp[i+3])) & 0xff) == 'R') {
>>>> vpd_base =
>>>> (int)((GET_ROM8(&(hmep->hme_romp[i+8])) & 0xff) |
>>>> (GET_ROM8(&(hmep->hme_romp[i+9])) & 0xff) << 8);
>>>> break; /* VPD pointer found */
>>>> }
>>>>
>>>>
>>>>
>>>Accept. Apart from the Cstyle indentation (which admittedly is
>>>weird, but it was weird before I touched it), there isn't really any
>>>runtime difference. The code actually passes cstyle -cPp as it
>>>stands. (What I did was make the minimal changes needed to pass
>>>Cstyle. I didn't want to get into restructuring code too much...
>>>otherwise there are far far worse places in this code.) I'm changing
>>>it anyway. (I think I did it this way when I made the same change in
>>>eri.)
>>>
>>>
>>Also note that my suggested change also includes comparing ascii
>>characters with ... get this ... ascii characters! :-) Go figure.
>>The hex values with little comments explaining which ascii characters
>>they mapped to was an especially nice touch in the old code. :-)
>>
>>
>
>Heh. Yeah. I made the same change in eri.c IIRC. When hme I took a
>more conservative approach in my changes than I did in eri.
>
>
For the uninitiated, what does "PCIR" represent?
Neither the old code or the new code explains what is being
done here and nor is it obvious?
Darren
_______________________________________________
driver-discuss mailing list
driver-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/driver-discuss
