linux-kernel - Re: [PATCH 2/3] usb gadget: don't save bind callback in struct usb_composite

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 09 Aug 2010 17:36:47 +0200
From:	Michał Nazarewicz <m.nazarewicz@...sung.com>
To:	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...e.de>
Subject: Re: [PATCH 2/3] usb gadget: don't save bind callback in struct
 usb_composite_driver

> On Mon, Aug 09, 2010 at 03:33:49PM +0200, Michal Nazarewicz wrote:
>> The bind function is most of the time only called at init time so there
>> is no need to save a pointer to it in the composite driver structure.
>>
>> This fixes many section mismatches reported by modpost.

>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -39,6 +39,7 @@
>>  #define USB_BUFSIZ	1024
>>
>>  static struct usb_composite_driver *composite;
>> +static int (*composite_gadget_bind)(struct usb_composite_dev *cdev);

On Mon, 09 Aug 2010 17:20:10 +0200, Uwe Kleine-König wrote:
> This can be __initdata, can't it?

No.  g_ffs.c uses usb_composite_probe() not only in init so neither
usb_composite_probe() nor this new variable can be in init section.

> I thought about usb_composite_drivers, too, and wondered if it would be
> sensible to change the additional argument of usb_gadget_probe_driver to
> struct usb_gadget_probe_data *probe_data.  Then you could do the
> following
>
> 	struct usb_composite_probe_data {
> 		struct usb_gadget_probe_data usb_probe_data;
> 		int (*composite_gadget_bind)(struct usb_composite_dev *);
> 	}
>
> and in composite_bind use container_of to get the gadget_bind function.

Yes, this seems sensible, however I'm not sure if it's worth the effort.
Note that the whole gadget API is designed in a way that there can be
only one gadget registered so that passing private data with
usb_gadget_probe_driver() as well as with usb_composite_probe() is not
really crucial and we can manage with global variables.

> Maybe that's overengineered though.  As there is already a dependency on
> a global variable (i.e. composite) your patch doesn't make it much
> worse.  Just my 0.02€.

Yep, that was my reasoning as well.

I guess we'll have to worry about it when (if) boards with many UDCs
became common. ;)

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/