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. :-)
-Seb
_______________________________________________
driver-discuss mailing list
driver-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/driver-discuss
