From: Behan Webster <[email protected]>
The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
precludes the use of compilers which don't implement VLAIS (for instance the
Clang compiler). This patch removes the use of VLAIS in the gadget driver.
Signed-off-by: Mark Charlebois <[email protected]>
Signed-off-by: Behan Webster <[email protected]>
---
drivers/usb/gadget/f_fs.c | 128 +++++++++++++++++++++++++++-------------------
1 file changed, 76 insertions(+), 52 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index f394f29..4b872c4 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -30,7 +30,6 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
-
/* Debugging ****************************************************************/
#ifdef VERBOSE_DEBUG
@@ -214,6 +213,8 @@ struct ffs_data {
/* ids in stringtabs are set in functionfs_bind() */
const void *raw_strings;
struct usb_gadget_strings **stringtabs;
+ struct usb_gadget_strings *stringtab;
+ struct usb_string *strings;
/*
* File system's super block, write once when file system is
@@ -263,7 +264,10 @@ struct ffs_function {
struct ffs_ep *eps;
u8 eps_revmap[16];
+ struct usb_descriptor_header **fs_descs;
+ struct usb_descriptor_header **hs_descs;
short *interfaces_nums;
+ char *raw_descs;
struct usb_function function;
};
@@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
kfree(ffs->raw_descs);
kfree(ffs->raw_strings);
kfree(ffs->stringtabs);
+ kfree(ffs->stringtab);
+ kfree(ffs->strings);
}
static void ffs_data_reset(struct ffs_data *ffs)
@@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
ffs->raw_descs = NULL;
ffs->raw_strings = NULL;
ffs->stringtabs = NULL;
+ ffs->stringtab = NULL;
+ ffs->strings = NULL;
ffs->raw_descs_length = 0;
ffs->raw_fs_descs_length = 0;
@@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
ffs_data_put(func->ffs);
kfree(func->eps);
- /*
- * eps and interfaces_nums are allocated in the same chunk so
- * only one free is required. Descriptors are also allocated
- * in the same chunk.
- */
-
+ kfree(func->fs_descs);
+ kfree(func->hs_descs);
+ kfree(func->interfaces_nums);
+ kfree(func->raw_descs);
kfree(func);
}
@@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
return 0;
}
- /* 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;
-
- d = kmalloc(sizeof *d, GFP_KERNEL);
- if (unlikely(!d)) {
+ usb_gadget_strings **stringtabs = NULL;
+ usb_gadget_strings *stringtab = NULL;
+ usb_string *strings = NULL;
+
+ stringtabs = kmalloc(sizeof(*stringtabs)*(lang_count + 1),
+ GFP_KERNEL);
+ stringtab = kmalloc(sizeof(*stringtab)*(lang_count),
+ GFP_KERNEL);
+ strings = kmalloc(sizeof(*strings)
+ * (lang_count * (needed_count + 1)), GFP_KERNEL);
+ if (unlikely(!stringtabs || !stringtab || !strings)) {
+ kfree(stringtabs);
+ kfree(stringtab);
+ kfree(strings);
kfree(_data);
return -ENOMEM;
}
-
- stringtabs = d->stringtabs;
- t = d->stringtab;
+ b = stringtabs;
+ t = stringtab;
i = lang_count;
do {
- *stringtabs++ = t++;
+ *b++ = t++;
} while (--i);
- *stringtabs = NULL;
+ *b = NULL;
- stringtabs = d->stringtabs;
- t = d->stringtab;
- s = d->strings;
- strings = s;
+ t = stringtab;
+ s = strings;
}
/* For each language */
@@ -1991,12 +1999,16 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
/* Done! */
ffs->stringtabs = stringtabs;
+ ffs->stringtab = stringtab;
+ ffs->strings = strings;
ffs->raw_strings = _data;
return 0;
error_free:
kfree(stringtabs);
+ kfree(stringtab);
+ kfree(strings);
error:
kfree(_data);
return -EINVAL;
@@ -2207,17 +2219,12 @@ static int ffs_func_bind(struct usb_configuration *c,
int ret;
- /* Make it a single chunk, less management later on */
- struct {
- struct ffs_ep eps[ffs->eps_count];
- struct usb_descriptor_header
- *fs_descs[full ? ffs->fs_descs_count + 1 : 0];
- struct usb_descriptor_header
- *hs_descs[high ? ffs->hs_descs_count + 1 : 0];
- short inums[ffs->interfaces_count];
- char raw_descs[high ? ffs->raw_descs_length
- : ffs->raw_fs_descs_length];
- } *data;
+ struct ffs_ep *eps = NULL;
+ struct usb_descriptor_header **fs_descs = NULL;
+ struct usb_descriptor_header **hs_descs = NULL;
+ short *inums = NULL;
+ char *raw_descs = NULL;
+
ENTER();
@@ -2225,21 +2232,40 @@ static int ffs_func_bind(struct usb_configuration *c,
if (unlikely(!(full | high)))
return -ENOTSUPP;
- /* Allocate */
- data = kmalloc(sizeof *data, GFP_KERNEL);
- if (unlikely(!data))
+ size_t eps_sz = sizeof(*eps)*ffs->eps_count;
+ eps = kmalloc(eps_sz, GFP_KERNEL);
+ fs_descs = kmalloc(sizeof(*fs_descs)*
+ (full ? ffs->fs_descs_count + 1 : 0), GFP_KERNEL);
+ hs_descs = kmalloc(sizeof(*hs_descs)*
+ (high ? ffs->hs_descs_count + 1 : 0), GFP_KERNEL);
+ size_t inums_sz = sizeof(*inums)*ffs->interfaces_count;
+ inums = kmalloc(inums_sz, GFP_KERNEL);
+ size_t raw_descs_sz = sizeof(*raw_descs)*(high ? ffs->raw_descs_length :
+ ffs->raw_fs_descs_length);
+ raw_descs = kmalloc(raw_descs_sz, , GFP_KERNEL);
+
+ if (unlikely(!eps || !fs_descs || !hs_descs || !inums || !raw_descs)) {
+ kfree(eps);
+ kfree(fs_descs);
+ kfree(hs_descs);
+ kfree(inums);
+ kfree(raw_descs);
return -ENOMEM;
+ }
/* Zero */
- memset(data->eps, 0, sizeof data->eps);
- memcpy(data->raw_descs, ffs->raw_descs + 16, sizeof data->raw_descs);
- memset(data->inums, 0xff, sizeof data->inums);
+ memset(eps, 0, eps_sz);
+ memcpy(raw_descs, ffs->raw_descs + 16, raw_descs_sz);
+ memset(inums, 0xff, inums_sz);
for (ret = ffs->eps_count; ret; --ret)
- data->eps[ret].num = -1;
+ eps[ret].num = -1;
/* Save pointers */
- func->eps = data->eps;
- func->interfaces_nums = data->inums;
+ func->eps = eps;
+ func->fs_descs = fs_descs;
+ func->hs_descs = hs_descs;
+ func->interfaces_nums = inums;
+ func->raw_descs = raw_descs;
/*
* Go through all the endpoint descriptors and allocate
@@ -2247,10 +2273,9 @@ static int ffs_func_bind(struct usb_configuration *c,
* numbers without worrying that it may be described later on.
*/
if (likely(full)) {
- func->function.fs_descriptors = data->fs_descs;
+ func->function.fs_descriptors = fs_descs;
ret = ffs_do_descs(ffs->fs_descs_count,
- data->raw_descs,
- sizeof data->raw_descs,
+ raw_descs, raw_descs_sz,
__ffs_func_bind_do_descs, func);
if (unlikely(ret < 0))
goto error;
@@ -2259,10 +2284,9 @@ static int ffs_func_bind(struct usb_configuration *c,
}
if (likely(high)) {
- func->function.hs_descriptors = data->hs_descs;
+ func->function.hs_descriptors = hs_descs;
ret = ffs_do_descs(ffs->hs_descs_count,
- data->raw_descs + ret,
- (sizeof data->raw_descs) - ret,
+ raw_descs + ret, raw_descs_sz - ret,
__ffs_func_bind_do_descs, func);
}
@@ -2273,7 +2297,7 @@ static int ffs_func_bind(struct usb_configuration *c,
*/
ret = ffs_do_descs(ffs->fs_descs_count +
(high ? ffs->hs_descs_count : 0),
- data->raw_descs, sizeof data->raw_descs,
+ raw_descs, raw_descs_sz,
__ffs_func_bind_do_nums, func);
if (unlikely(ret < 0))
goto error;
--
1.8.1.2
Replying to my patch email just in case it was missed before.
Thanks,
Behan
On 08/01/13 21:35, [email protected] wrote:
> From: Behan Webster <[email protected]>
>
> The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
> precludes the use of compilers which don't implement VLAIS (for instance the
> Clang compiler). This patch removes the use of VLAIS in the gadget driver.
>
> Signed-off-by: Mark Charlebois <[email protected]>
> Signed-off-by: Behan Webster <[email protected]>
> ---
> drivers/usb/gadget/f_fs.c | 128 +++++++++++++++++++++++++++-------------------
> 1 file changed, 76 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index f394f29..4b872c4 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -30,7 +30,6 @@
>
> #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
>
> -
> /* Debugging ****************************************************************/
>
> #ifdef VERBOSE_DEBUG
> @@ -214,6 +213,8 @@ struct ffs_data {
> /* ids in stringtabs are set in functionfs_bind() */
> const void *raw_strings;
> struct usb_gadget_strings **stringtabs;
> + struct usb_gadget_strings *stringtab;
> + struct usb_string *strings;
>
> /*
> * File system's super block, write once when file system is
> @@ -263,7 +264,10 @@ struct ffs_function {
>
> struct ffs_ep *eps;
> u8 eps_revmap[16];
> + struct usb_descriptor_header **fs_descs;
> + struct usb_descriptor_header **hs_descs;
> short *interfaces_nums;
> + char *raw_descs;
>
> struct usb_function function;
> };
> @@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
> kfree(ffs->raw_descs);
> kfree(ffs->raw_strings);
> kfree(ffs->stringtabs);
> + kfree(ffs->stringtab);
> + kfree(ffs->strings);
> }
>
> static void ffs_data_reset(struct ffs_data *ffs)
> @@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
> ffs->raw_descs = NULL;
> ffs->raw_strings = NULL;
> ffs->stringtabs = NULL;
> + ffs->stringtab = NULL;
> + ffs->strings = NULL;
>
> ffs->raw_descs_length = 0;
> ffs->raw_fs_descs_length = 0;
> @@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
> ffs_data_put(func->ffs);
>
> kfree(func->eps);
> - /*
> - * eps and interfaces_nums are allocated in the same chunk so
> - * only one free is required. Descriptors are also allocated
> - * in the same chunk.
> - */
> -
> + kfree(func->fs_descs);
> + kfree(func->hs_descs);
> + kfree(func->interfaces_nums);
> + kfree(func->raw_descs);
> kfree(func);
> }
>
> @@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
> return 0;
> }
>
> - /* 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;
> -
> - d = kmalloc(sizeof *d, GFP_KERNEL);
> - if (unlikely(!d)) {
> + usb_gadget_strings **stringtabs = NULL;
> + usb_gadget_strings *stringtab = NULL;
> + usb_string *strings = NULL;
> +
> + stringtabs = kmalloc(sizeof(*stringtabs)*(lang_count + 1),
> + GFP_KERNEL);
> + stringtab = kmalloc(sizeof(*stringtab)*(lang_count),
> + GFP_KERNEL);
> + strings = kmalloc(sizeof(*strings)
> + * (lang_count * (needed_count + 1)), GFP_KERNEL);
> + if (unlikely(!stringtabs || !stringtab || !strings)) {
> + kfree(stringtabs);
> + kfree(stringtab);
> + kfree(strings);
> kfree(_data);
> return -ENOMEM;
> }
> -
> - stringtabs = d->stringtabs;
> - t = d->stringtab;
> + b = stringtabs;
> + t = stringtab;
> i = lang_count;
> do {
> - *stringtabs++ = t++;
> + *b++ = t++;
> } while (--i);
> - *stringtabs = NULL;
> + *b = NULL;
>
> - stringtabs = d->stringtabs;
> - t = d->stringtab;
> - s = d->strings;
> - strings = s;
> + t = stringtab;
> + s = strings;
> }
>
> /* For each language */
> @@ -1991,12 +1999,16 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
>
> /* Done! */
> ffs->stringtabs = stringtabs;
> + ffs->stringtab = stringtab;
> + ffs->strings = strings;
> ffs->raw_strings = _data;
>
> return 0;
>
> error_free:
> kfree(stringtabs);
> + kfree(stringtab);
> + kfree(strings);
> error:
> kfree(_data);
> return -EINVAL;
> @@ -2207,17 +2219,12 @@ static int ffs_func_bind(struct usb_configuration *c,
>
> int ret;
>
> - /* Make it a single chunk, less management later on */
> - struct {
> - struct ffs_ep eps[ffs->eps_count];
> - struct usb_descriptor_header
> - *fs_descs[full ? ffs->fs_descs_count + 1 : 0];
> - struct usb_descriptor_header
> - *hs_descs[high ? ffs->hs_descs_count + 1 : 0];
> - short inums[ffs->interfaces_count];
> - char raw_descs[high ? ffs->raw_descs_length
> - : ffs->raw_fs_descs_length];
> - } *data;
> + struct ffs_ep *eps = NULL;
> + struct usb_descriptor_header **fs_descs = NULL;
> + struct usb_descriptor_header **hs_descs = NULL;
> + short *inums = NULL;
> + char *raw_descs = NULL;
> +
>
> ENTER();
>
> @@ -2225,21 +2232,40 @@ static int ffs_func_bind(struct usb_configuration *c,
> if (unlikely(!(full | high)))
> return -ENOTSUPP;
>
> - /* Allocate */
> - data = kmalloc(sizeof *data, GFP_KERNEL);
> - if (unlikely(!data))
> + size_t eps_sz = sizeof(*eps)*ffs->eps_count;
> + eps = kmalloc(eps_sz, GFP_KERNEL);
> + fs_descs = kmalloc(sizeof(*fs_descs)*
> + (full ? ffs->fs_descs_count + 1 : 0), GFP_KERNEL);
> + hs_descs = kmalloc(sizeof(*hs_descs)*
> + (high ? ffs->hs_descs_count + 1 : 0), GFP_KERNEL);
> + size_t inums_sz = sizeof(*inums)*ffs->interfaces_count;
> + inums = kmalloc(inums_sz, GFP_KERNEL);
> + size_t raw_descs_sz = sizeof(*raw_descs)*(high ? ffs->raw_descs_length :
> + ffs->raw_fs_descs_length);
> + raw_descs = kmalloc(raw_descs_sz, , GFP_KERNEL);
> +
> + if (unlikely(!eps || !fs_descs || !hs_descs || !inums || !raw_descs)) {
> + kfree(eps);
> + kfree(fs_descs);
> + kfree(hs_descs);
> + kfree(inums);
> + kfree(raw_descs);
> return -ENOMEM;
> + }
>
> /* Zero */
> - memset(data->eps, 0, sizeof data->eps);
> - memcpy(data->raw_descs, ffs->raw_descs + 16, sizeof data->raw_descs);
> - memset(data->inums, 0xff, sizeof data->inums);
> + memset(eps, 0, eps_sz);
> + memcpy(raw_descs, ffs->raw_descs + 16, raw_descs_sz);
> + memset(inums, 0xff, inums_sz);
> for (ret = ffs->eps_count; ret; --ret)
> - data->eps[ret].num = -1;
> + eps[ret].num = -1;
>
> /* Save pointers */
> - func->eps = data->eps;
> - func->interfaces_nums = data->inums;
> + func->eps = eps;
> + func->fs_descs = fs_descs;
> + func->hs_descs = hs_descs;
> + func->interfaces_nums = inums;
> + func->raw_descs = raw_descs;
>
> /*
> * Go through all the endpoint descriptors and allocate
> @@ -2247,10 +2273,9 @@ static int ffs_func_bind(struct usb_configuration *c,
> * numbers without worrying that it may be described later on.
> */
> if (likely(full)) {
> - func->function.fs_descriptors = data->fs_descs;
> + func->function.fs_descriptors = fs_descs;
> ret = ffs_do_descs(ffs->fs_descs_count,
> - data->raw_descs,
> - sizeof data->raw_descs,
> + raw_descs, raw_descs_sz,
> __ffs_func_bind_do_descs, func);
> if (unlikely(ret < 0))
> goto error;
> @@ -2259,10 +2284,9 @@ static int ffs_func_bind(struct usb_configuration *c,
> }
>
> if (likely(high)) {
> - func->function.hs_descriptors = data->hs_descs;
> + func->function.hs_descriptors = hs_descs;
> ret = ffs_do_descs(ffs->hs_descs_count,
> - data->raw_descs + ret,
> - (sizeof data->raw_descs) - ret,
> + raw_descs + ret, raw_descs_sz - ret,
> __ffs_func_bind_do_descs, func);
> }
>
> @@ -2273,7 +2297,7 @@ static int ffs_func_bind(struct usb_configuration *c,
> */
> ret = ffs_do_descs(ffs->fs_descs_count +
> (high ? ffs->hs_descs_count : 0),
> - data->raw_descs, sizeof data->raw_descs,
> + raw_descs, raw_descs_sz,
> __ffs_func_bind_do_nums, func);
> if (unlikely(ret < 0))
> goto error;
--
Behan Webster
[email protected]
Hi,
On Thu, Sep 05, 2013 at 08:07:11PM -0400, Behan Webster wrote:
> Replying to my patch email just in case it was missed before.
I remember there was a discussion of not dropping variable length array
support, wasn't there ?
--
balbi
On 09/23/13 14:30, Felipe Balbi wrote:
> Hi,
>
> On Thu, Sep 05, 2013 at 08:07:11PM -0400, Behan Webster wrote:
>> Replying to my patch email just in case it was missed before.
> I remember there was a discussion of not dropping variable length array
> support, wasn't there ?
There was mostly an objection to the implementation of that patch. The
new patch follows the advice given by the linux-usb list members at that
time.
There is increasing interest from various kernel devs (including
maintainers and above) to be able to use clang to compile the Linux
kernel. The removal of the use of VLAIS in the kernel is a step in
allowing that to happen.
Andrzej Pietrasiewicz attended the LLVM microconference at Plumbers last
week and accepted the USB gadget patch.
Thanks,
Behan
--
Behan Webster
[email protected]
On Mon, Sep 23, 2013 at 12:30 PM, Felipe Balbi <[email protected]> wrote:
>
> I remember there was a discussion of not dropping variable length array
> support, wasn't there ?
We should definitely drop it. The feature is an abomination. I thought
gcc only allowed them at the end of structs, in the middle of a struct
it's just f*cking insane beyond belief.
That said, for *this* particular case, that USB widget driver already
does a ton of small kmalloc's and then remembers the addresses
independently. People may not care about performance, but it *might*
be a good idea to just do one kmalloc()/kfree(), and then still have
those pointer variables, but just be offsets within that one
allocation.
That's what gcc has to basically do for that thing internally
*anyway*, just hidden behind a horrible construct that should never
have existed.
Linus
On 09/23/13 15:08, Linus Torvalds wrote:
> On Mon, Sep 23, 2013 at 12:30 PM, Felipe Balbi <[email protected]> wrote:
>> I remember there was a discussion of not dropping variable length array
>> support, wasn't there ?
> We should definitely drop it. The feature is an abomination. I thought
> gcc only allowed them at the end of structs, in the middle of a struct
> it's just f*cking insane beyond belief.
>
> That said, for *this* particular case, that USB widget driver already
> does a ton of small kmalloc's and then remembers the addresses
> independently. People may not care about performance, but it *might*
> be a good idea to just do one kmalloc()/kfree(), and then still have
> those pointer variables, but just be offsets within that one
> allocation.
>
> That's what gcc has to basically do for that thing internally
> *anyway*, just hidden behind a horrible construct that should never
> have existed.
We can certainly do that instead.
I believe I already have a version of the patch which does just that
(without using macros). I will post it for comment.
Thanks,
Behan
--
Behan Webster
[email protected]
Hi,
On Thu, Aug 01, 2013 at 09:35:44PM -0400, [email protected] wrote:
> From: Behan Webster <[email protected]>
>
> The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
> precludes the use of compilers which don't implement VLAIS (for instance the
> Clang compiler). This patch removes the use of VLAIS in the gadget driver.
>
> Signed-off-by: Mark Charlebois <[email protected]>
> Signed-off-by: Behan Webster <[email protected]>
> ---
> drivers/usb/gadget/f_fs.c | 128 +++++++++++++++++++++++++++-------------------
> 1 file changed, 76 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index f394f29..4b872c4 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -30,7 +30,6 @@
>
> #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
>
> -
> /* Debugging ****************************************************************/
>
> #ifdef VERBOSE_DEBUG
> @@ -214,6 +213,8 @@ struct ffs_data {
> /* ids in stringtabs are set in functionfs_bind() */
> const void *raw_strings;
> struct usb_gadget_strings **stringtabs;
> + struct usb_gadget_strings *stringtab;
> + struct usb_string *strings;
>
> /*
> * File system's super block, write once when file system is
> @@ -263,7 +264,10 @@ struct ffs_function {
>
> struct ffs_ep *eps;
> u8 eps_revmap[16];
> + struct usb_descriptor_header **fs_descs;
> + struct usb_descriptor_header **hs_descs;
> short *interfaces_nums;
> + char *raw_descs;
>
> struct usb_function function;
> };
> @@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
> kfree(ffs->raw_descs);
> kfree(ffs->raw_strings);
> kfree(ffs->stringtabs);
> + kfree(ffs->stringtab);
> + kfree(ffs->strings);
> }
>
> static void ffs_data_reset(struct ffs_data *ffs)
> @@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
> ffs->raw_descs = NULL;
> ffs->raw_strings = NULL;
> ffs->stringtabs = NULL;
> + ffs->stringtab = NULL;
> + ffs->strings = NULL;
>
> ffs->raw_descs_length = 0;
> ffs->raw_fs_descs_length = 0;
> @@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
> ffs_data_put(func->ffs);
>
> kfree(func->eps);
> - /*
> - * eps and interfaces_nums are allocated in the same chunk so
> - * only one free is required. Descriptors are also allocated
> - * in the same chunk.
> - */
> -
> + kfree(func->fs_descs);
> + kfree(func->hs_descs);
> + kfree(func->interfaces_nums);
> + kfree(func->raw_descs);
> kfree(func);
> }
>
> @@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
> return 0;
> }
>
> - /* 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;
> -
> - d = kmalloc(sizeof *d, GFP_KERNEL);
> - if (unlikely(!d)) {
> + usb_gadget_strings **stringtabs = NULL;
> + usb_gadget_strings *stringtab = NULL;
> + usb_string *strings = NULL;
did you compile this patch ?
--
balbi
On 09/23/13 16:24, Felipe Balbi wrote:
> Hi,
>
> On Thu, Aug 01, 2013 at 09:35:44PM -0400, [email protected] wrote:
>> From: Behan Webster <[email protected]>
>>
>> The use of variable length arrays in structs (VLAIS) in the Linux Kernel code
>> precludes the use of compilers which don't implement VLAIS (for instance the
>> Clang compiler). This patch removes the use of VLAIS in the gadget driver.
>>
>> Signed-off-by: Mark Charlebois <[email protected]>
>> Signed-off-by: Behan Webster <[email protected]>
>> ---
>> drivers/usb/gadget/f_fs.c | 128 +++++++++++++++++++++++++++-------------------
>> 1 file changed, 76 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
>> index f394f29..4b872c4 100644
>> --- a/drivers/usb/gadget/f_fs.c
>> +++ b/drivers/usb/gadget/f_fs.c
>> @@ -30,7 +30,6 @@
>>
>> #define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
>>
>> -
>> /* Debugging ****************************************************************/
>>
>> #ifdef VERBOSE_DEBUG
>> @@ -214,6 +213,8 @@ struct ffs_data {
>> /* ids in stringtabs are set in functionfs_bind() */
>> const void *raw_strings;
>> struct usb_gadget_strings **stringtabs;
>> + struct usb_gadget_strings *stringtab;
>> + struct usb_string *strings;
>>
>> /*
>> * File system's super block, write once when file system is
>> @@ -263,7 +264,10 @@ struct ffs_function {
>>
>> struct ffs_ep *eps;
>> u8 eps_revmap[16];
>> + struct usb_descriptor_header **fs_descs;
>> + struct usb_descriptor_header **hs_descs;
>> short *interfaces_nums;
>> + char *raw_descs;
>>
>> struct usb_function function;
>> };
>> @@ -1345,6 +1349,8 @@ static void ffs_data_clear(struct ffs_data *ffs)
>> kfree(ffs->raw_descs);
>> kfree(ffs->raw_strings);
>> kfree(ffs->stringtabs);
>> + kfree(ffs->stringtab);
>> + kfree(ffs->strings);
>> }
>>
>> static void ffs_data_reset(struct ffs_data *ffs)
>> @@ -1357,6 +1363,8 @@ static void ffs_data_reset(struct ffs_data *ffs)
>> ffs->raw_descs = NULL;
>> ffs->raw_strings = NULL;
>> ffs->stringtabs = NULL;
>> + ffs->stringtab = NULL;
>> + ffs->strings = NULL;
>>
>> ffs->raw_descs_length = 0;
>> ffs->raw_fs_descs_length = 0;
>> @@ -1528,12 +1536,10 @@ static void ffs_func_free(struct ffs_function *func)
>> ffs_data_put(func->ffs);
>>
>> kfree(func->eps);
>> - /*
>> - * eps and interfaces_nums are allocated in the same chunk so
>> - * only one free is required. Descriptors are also allocated
>> - * in the same chunk.
>> - */
>> -
>> + kfree(func->fs_descs);
>> + kfree(func->hs_descs);
>> + kfree(func->interfaces_nums);
>> + kfree(func->raw_descs);
>> kfree(func);
>> }
>>
>> @@ -1907,33 +1913,35 @@ static int __ffs_data_got_strings(struct ffs_data *ffs,
>> return 0;
>> }
>>
>> - /* 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;
>> -
>> - d = kmalloc(sizeof *d, GFP_KERNEL);
>> - if (unlikely(!d)) {
>> + usb_gadget_strings **stringtabs = NULL;
>> + usb_gadget_strings *stringtab = NULL;
>> + usb_string *strings = NULL;
> did you compile this patch ?
>
Appologies. The patch as posted has a bug which was fixed after sending
it to the list.
Andrzej Pietrasiewicz <[email protected]> has the fixed one. Will
send the fixed one to the list too.
Behan
--
Behan Webster
[email protected]