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