By the way, I found a few more warnings in lint because of this change.  
Essentially, MBLKL() now returns an unsigned value, rather than signed, 
because of casting to (uintptr_t).  (And that's pretty reasonable... the 
idea that there could ever be an mblk where b_wptr < b_rptr is nonsensical.)

Upshot of this is that in some of the USB serial drivers, there were 
constructs like MBLKL(mp) <= 0.  I've changed those to read MBLKL(mp) < 
1.  This has the exactly same semantics, and works regardless of the 
return type of MBLKL, without causing E_SUSPICIOUS_COMPARISION warnings 
from lint.  (Essentially lint understands that an unsigned type can 
*never* have a value less than 0.)

The internal webrev is at http://jurassic.sfbay/~gd78059/webrev/mblkl

I've not bothered to generate a public webrev, as the changes were small 
and the reviewers that have responded are all internal Sun folks.  If 
anyone wants a public webrev, let me know.

That said, here are the lint warnings that these fixes correct (so you 
don't need to re-review all the changes):

"../../common/io/usb/clients/usbser/usbsacm/usbsacm.c", line 1285: warning: 
suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbsacm/usbsacm.c", line 2453: warning: 
suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbser.c", line 2556: warning: suspicious 
comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbser_edge/edge_dsd.c", line 1675: 
warning: suspicious comparison of unsigned with 0: op "<=" 
(E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbser_edge/edge_dsd.c", line 823: warning: 
suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbser_keyspan/keyspan_dsd.c", line 2508: 
warning: suspicious comparison of unsigned with 0: op "<=" 
(E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbsprl/pl2303_dsd.c", line 1741: warning: 
suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)
"../../common/io/usb/clients/usbser/usbsprl/pl2303_dsd.c", line 919: warning: 
suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON)


In all other respects the nightly build was clean. :-)  I'm hoping to 
submit my RTI in the next 24 hours.  I want to get a clean sparc build 
and do some spot tests including bfu'ing the bits over to a machine first.

    -- Garrett

Garrett D'Amore wrote:
> I think Jim already addressed this, which is basically that this becomes 
> a matter of personal taste to a certain extent.  (Meaning /*LINTED*/ 
> versus (void *).)
>
> One rationale for using (void *) is that it mirrors our use of the 
> source pointer in these cases -- that is, we are using the source 
> pointer as an untyped pointer (e.g. mp->b_rptr), so it makes sense to 
> use an untyped cast ... i.e. void *.  It does have the possibility that 
> it obfuscates the type of the destination pointer, but I think good 
> programming practice is that you use names such that the type is more or 
> less obvious -- though I'm *NOT* advocating Hungarian notation.
>
> Anyway, are you happy with the other changes, and can I list you as a 
> reviewer on the RTI?  I want to submit this RTI as soon as possible, as 
> my advocate has requested this be submitted asap as he wants to avoid 
> introducing further lint breakage in another putback he's working on. :-)
>
>     -- Garrett
>
> Tzongyu Paul Lee wrote:
>   
>> James Carlson 写道:
>>     
>>> Garrett D'Amore writes:
>>>  
>>>       
>>>> James Carlson wrote:
>>>>    
>>>>         
>>>>> Garrett D'Amore writes:
>>>>>        
>>>>>           
>>>>>> There is a closed version, which includes one closed file fix, at
>>>>>>
>>>>>> http://jurassic.sfbay/~gd78059/webrev/mblkl
>>>>>>         
>>>>>>             
>>> "OK" on all the responses.
>>>   
>>>       
>> I am uncomfortable with some of the (void *) changes, for example,
>> closed/uts/common/io/ib/clients/sdpib/sdp_link.c
>>
>> from
>> 102 /* LINTED */
>> 103 ipllc = (ipllc_t *)mp->b_rptr;
>> 104 ipllc->ipllc_cmd = IP_IOC_RTS_REQUEST;
>>
>> to
>>
>> 102 ipllc = (void *)mp->b_rptr;
>>
>> 103 ipllc->ipllc_cmd = IP_IOC_RTS_REQUEST;
>>
>> Although (void *) makes lint happy, this change
>> greatly dilutes the clarity of a structure pointer which
>> gets used immediately to access member fields in that structure.
>> I feel that we are bending too much to please the lint checker.
>>     
>>>  
>>>       
>>>>> uts/intel/pcwl/Makefile
>>>>>
>>>>>   75: remove
>>>>>         
>>>>>           
>>>> Huh?  You better clarify that one for me.
>>>>     
>>>>         
>>> Instead of actually removing the now-unused LINTTAGS += entry in this
>>> one Makefile, you just commented it out.  I think it'd be better to
>>> remove the line.  If anyone really needs it in the future, they'll
>>> have to come up with the recipe on their own.
>>>
>>>   
>>>       
>
> _______________________________________________
> driver-discuss mailing list
> [EMAIL PROTECTED]
> http://mail.opensolaris.org/mailman/listinfo/driver-discuss
>   

_______________________________________________
opensolaris-code mailing list
opensolaris-code@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code

Reply via email to