On Thu, Jun 14, 2012 at 10:03:02AM -0400, Alan Stern wrote:
> On Wed, 13 Jun 2012, Greg KH wrote:
>
> > > > > +struct usb_hub_port {
> > > > > + void *port_owner;
> > > >
> > > > What? Why a void pointer? You should almost _never_ use a void pointer
> > > > in kernel code, it's bad form, and it means you are trying to hide
> > > > something here, when it shouldn't be hidden, as we should know the real
> > > > type, right?
> > >
> > > That's my fault; Lan is simply moving code that I wrote.
> > >
> > > The void * values are simply cookies as far as the hub driver is
> > > concerned -- the meaning is _intended_ to be hidden and the pointers
> > > are never dereferenced. All that matters about them is whether two
> > > values are the same or different. The actual pointers stored in
> > > port_owner are created by usbfs; they are pointers to the internal data
> > > structures defined in devio.c.
> > >
> > > If you really want, we could change void * to struct dev_state *. I'm
> > > not sure this would make things any better or clearer.
> >
> > Yes, it would make it clearer when we read the code, seing dev_state *,
> > makes me know that we know what is going on. A void * makes me know we
> > have no clue as to what is going on :)
>
> Hmmm. The thing is, these values really aren't pointers -- or at
> least, they aren't used as pointers.
What are they used for then?
> What's the right type to represent an arbitrary cookie? Something
> large enough to hold a pointer but which will never be dereferenced?
> Unsigned long? Or maybe a typedef:
If it's not going to be used as a pointer, then what is this going to be
used as?
> typedef void *hub_cookie_t;
>
> What do you suggest?
struct usb_hub_cookie {
unsigned long cookie;
};
Never use _t in new kernel code :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
