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'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:
typedef void *hub_cookie_t;
What do you suggest?
Alan Stern
--
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
