Hello Sebastian,

thank you for review, please see inline

W dniu 08.11.2013 09:02, Sebastian Andrzej Siewior pisze:
On 10/23/2013 02:25 PM, Andrzej Pietrasiewicz wrote:
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 0658908..9ccda73 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -1901,30 +1916,38 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,

        /* Allocate everything in one chunk so there's less maintenance. */
        {
-               struct {
-                       struct usb_gadget_strings *stringtabs[lang_count + 1];
-                       struct usb_gadget_strings stringtab[lang_count];
-                       struct usb_string strings[lang_count*(needed_count+1)];
-               } *d;
                unsigned i = 0;
+               vla_group(d);
+               vla_item(d, struct usb_gadget_strings *, stringtabs,
+                       lang_count + 1);
+               vla_item(d, struct usb_gadget_strings, stringtab, lang_count);
+               vla_item(d, struct usb_string, strings,
+                       lang_count*(needed_count+1));

-               d = kmalloc(sizeof *d, GFP_KERNEL);
-               if (unlikely(!d)) {
+               char *vlabuf = kmalloc(vla_group_size(d), GFP_KERNEL);
+
+               if (unlikely(!vlabuf)) {
                        kfree(_data);
                        return -ENOMEM;
                }

-               stringtabs = d->stringtabs;
-               t = d->stringtab;
+               /* Initialize the VLA pointers */
+               vla_ptr(vlabuf, d, stringtabs);
+               vla_ptr(vlabuf, d, stringtab);
+               vla_ptr(vlabuf, d, strings);
+
+               stringtabs = d_stringtabs;
+               t = d_stringtab;
                i = lang_count;
                do {
                        *stringtabs++ = t++;
                } while (--i);
                *stringtabs = NULL;

-               stringtabs = d->stringtabs;
-               t = d->stringtab;
-               s = d->strings;
+               /* stringtabs = vlabuf = d_stringtabs for later kfree */
+               stringtabs = d_stringtabs;
+               t = d_stringtab;
+               s = d_strings;
                strings = s;
        }


Couldn't you use usb_gstrings_attach() instead? It should do the same
job without those macros.


Please correct me if I'm wrong but I think it is not possible to use usb_gstrings_attach(). Basically, usb_gstrings_attach() requires

struct usb_gadget_strings **sp

on input, then it does copy_gadget_strings(sp, n_gstrings, n_strings);
and then does its job. So actually to do usb_gstrings_attach() you need
to _already_ have struct usb_gadget_strings **sp.

FunctionFS is different in that it takes data to _build_ usb_gadget_strings **stringtabs from userspace (the char *const _data).
In other words, the string descriptors are provided from userspace as
a character buffer which needs to be parsed and it is what
__ffs_data_got_strings() does.

Andrzej


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to