linux-kernel - Re: [PATCH v5 2/5] usb: gadget: configfs: pass UDC name via usb_gadget

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] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 27 Jun 2015 17:33:41 +0300
From:	Ruslan Bilovol <ruslan.bilovol@...il.com>
To:	Krzysztof Opasiak <k.opasiak@...sung.com>
Cc:	"Balbi, Felipe" <balbi@...com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Peter Chen <peter.chen@...escale.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	Andrzej Pietrasiewicz <andrzej.p@...sung.com>,
	Maxime Ripard <maxime.ripard@...e-electrons.com>
Subject: Re: [PATCH v5 2/5] usb: gadget: configfs: pass UDC name via
 usb_gadget_driver struct

Hi Krzysztof,

On Tue, Jun 23, 2015 at 9:54 AM, Krzysztof Opasiak
<k.opasiak@...sung.com> wrote:
> Hello,
>
> On 06/23/2015 12:01 AM, Ruslan Bilovol wrote:
>>
>> Now when udc-core supports binding to specific UDC by passing
>> its name via 'udc_name' member of usb_gadget_driver struct,
>> switch to this generic approach.
>>
>> Tested-by: Maxime Ripard <maxime.ripard@...e-electrons.com>
>> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@...il.com>
>> ---
>>   drivers/usb/gadget/configfs.c | 27 ++++++++++++++-------------
>>   1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
>> index c42765b..efad021 100644
>> --- a/drivers/usb/gadget/configfs.c
>> +++ b/drivers/usb/gadget/configfs.c
>> @@ -54,7 +54,6 @@ struct gadget_info {
>>         struct list_head string_list;
>>         struct list_head available_func;
>>
>> -       const char *udc_name;
>>   #ifdef CONFIG_USB_OTG
>>         struct usb_otg_descriptor otg;
>>   #endif
>> @@ -230,21 +229,21 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct
>> gadget_info *gi,
>>
>>   static ssize_t gadget_dev_desc_UDC_show(struct gadget_info *gi, char
>> *page)
>>   {
>> -       return sprintf(page, "%s\n", gi->udc_name ?: "");
>> +       return sprintf(page, "%s\n", gi->composite.gadget_driver.udc_name
>> ?: "");
>>   }
>>
>>   static int unregister_gadget(struct gadget_info *gi)
>>   {
>>         int ret;
>>
>> -       if (!gi->udc_name)
>> +       if (!gi->composite.gadget_driver.udc_name)
>>                 return -ENODEV;
>>
>>         ret = usb_gadget_unregister_driver(&gi->composite.gadget_driver);
>>         if (ret)
>>                 return ret;
>> -       kfree(gi->udc_name);
>> -       gi->udc_name = NULL;
>> +       kfree(gi->composite.gadget_driver.udc_name);
>> +       gi->composite.gadget_driver.udc_name = NULL;
>>         return 0;
>>   }
>>
>> @@ -267,14 +266,16 @@ static ssize_t gadget_dev_desc_UDC_store(struct
>> gadget_info *gi,
>>                 if (ret)
>>                         goto err;
>>         } else {
>> -               if (gi->udc_name) {
>> +               if (gi->composite.gadget_driver.udc_name) {
>
>
> You are using this very long if condition in a few places, maybe it would be
> more suitable to define a macro or inline function for this? Something like
> gadget_dev_bound() or gadget_dev_is_active() or some other more suitable
> name.

Yes, it makes sense, since there are 11 places where this long access
to udc_name is used.
However, it is used not only in if conditions but also for changing
udc_name, so need to add some getters/setters, something like
#define gi_to_udc_name(gi)
((gi)->composite.gadget_driver.udc_name)
#define gi_set_udc_name(gi,name)
((gi)->composite.gadget_driver.udc_name = (name))

Best regards,
Ruslan
--
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/