On Wed, Jun 13, 2012 at 05:22:48PM -0400, Alan Stern wrote:
> On Wed, 13 Jun 2012, Greg KH wrote:
>
> > On Wed, Jun 13, 2012 at 11:07:17AM +0800, Lan Tianyu wrote:
> > > Add struct usb_hub_port pointer port_data in the struct usb_hub and allocate
> > > struct usb_hub_port perspectively for each ports to store private data.
> > >
> > > Signed-off-by: Lan Tianyu <tianyu.lan@xxxxxxxxx>
> > > ---
> > > drivers/usb/core/hub.c | 29 ++++++++++++++++-------------
> > > 1 files changed, 16 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > index 04fb834..512f0d5 100644
> > > --- a/drivers/usb/core/hub.c
> > > +++ b/drivers/usb/core/hub.c
> > > @@ -37,6 +37,10 @@
> > > #endif
> > > #endif
> > >
> > > +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 :)
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
