Re: [PATCH] Remove VLAIS usage from gadget code - alternate patch
Hi Mark,
Nice to hear from you again; on Saturday LOT's dreamliner was not
grounded and I have safely returned home ;)
Please see my comments inline.
W dniu 24.09.2013 19:56, charl...@gmail.com pisze:
From: Mark Charlebois charl...@gmail.com
--- linux.orig/drivers/usb/gadget/f_fs.c
+++ linux/drivers/usb/gadget/f_fs.c
@@ -30,6 +30,21 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
+/* Variable Length Array Macros **/
+#define vla_struct(structname) size_t structname##__##next = 0
+#define vla_struct_size(structname) structname##__##next
+
+#define vla_item(structname, type, name, n) \
+ type * structname##_##name; \
+ size_t structname##_##name##__##offset = \
+ (structname##__##next + __alignof__(type) - 1) \
+ ~(__alignof__(type) - 1); \
+ size_t structname##_##name##__##sz = n * sizeof(type); \
most likely this shoud read:
+ size_t structname##_##name##__##sz = (n) * sizeof(type); \
otherwise vla_item(., lang_count + 1); will expand to:
size_t d_stringtabs__sz = lang_count + \
1 * sizeof(struct usb_gadget_strings *);
+ structname##__##next = structname##_##name##__##offset + \
+ structname##_##name##__##sz;
+
+#define vla_ptr(ptr,structname,name) structname##_##name = \
+ (__typeof__(structname##_##name))ptr[structname##_##name##__##offset]
snip
unsigned i = 0;
+ vla_struct(d);
+ vla_item(d, struct usb_gadget_strings *, stringtabs,
+ lang_count + 1);
Can you somehow avoid mixing code and declarations? The last thing in
the expansion of this vla_item(...) is an assignment, and
+ vla_item(d, struct usb_gadget_strings, stringtab, lang_count);
the first thing in expansion of the next vla_item(...) is a
declaration. GCC most likely will complain (issue a warning).
One solution I can think of here (a bit hackish) is to use a braced
group as an expression: define vla_item() in such a way that first
it declares e.g. d_stringtabs__offset, then d_stringtabs__sz, and
then
struct usb_gadget_strings **d_stringtabs =
({d__next = d_stringtabs__offset + d_stringtabs__sz; NULL;});
I am not a fan of this kind of style, but can't think of any better way
now. And I don't know what Clang thinks of it :O
Thanks,
AP
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Remove VLAIS usage from gadget code - alternate patch
From: Mark Charlebois charl...@gmail.com
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 alternate patch calculates offsets into the kmalloc-ed
memory buffer using macros. The previous patch required multiple kmalloc and
kfree calls. This version uses group vs struct since it really is not a
struct and is essentially a group of VLA in a common allocated block. This
version also fixes the issues pointed out by Andrzej Pietrasiewicz.
Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Behan Webster beh...@converseincode.com
---
--- linux.orig/drivers/usb/gadget/f_fs.c
+++ linux/drivers/usb/gadget/f_fs.c
@@ -30,6 +30,21 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
+/* Variable Length Array Macros **/
+#define vla_group(groupname) size_t groupname##__##next = 0
+#define vla_group_size(groupname) groupname##__##next
+
+#define vla_item(groupname, type, name, n) \
+ size_t groupname##_##name##__##offset = \
+ (groupname##__##next + __alignof__(type) - 1) \
+ ~(__alignof__(type) - 1); \
+ size_t groupname##_##name##__##sz = (n) * sizeof(type); \
+ type * groupname##_##name = ({ \
+ groupname##__##next = groupname##_##name##__##offset + \
+ groupname##_##name##__##sz; NULL;})
+
+#define vla_ptr(ptr,groupname,name) groupname##_##name = \
+ (__typeof__(groupname##_##name))ptr[groupname##_##name##__##offset]
/* Debugging /
@@ -1909,30 +1924,38 @@
/* 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));
+
+ char *vlabuf = kmalloc(vla_group_size(d), GFP_KERNEL);
- d = kmalloc(sizeof *d, GFP_KERNEL);
- if (unlikely(!d)) {
+ 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;
}
@@ -2208,16 +2231,15 @@
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;
+ vla_group(d);
+ vla_item(d, struct ffs_ep, eps, ffs-eps_count);
+ vla_item(d, struct usb_descriptor_header *, fs_descs,
+ full ? ffs-fs_descs_count + 1 : 0);
+ vla_item(d, struct usb_descriptor_header *, hs_descs,
+ high ? ffs-hs_descs_count + 1 : 0);
+ vla_item(d, short, inums, ffs-interfaces_count);
+ vla_item(d, char, raw_descs,
+ high ? ffs-raw_descs_length : ffs-raw_fs_descs_length);
ENTER();
@@ -2225,21 +2247,30 @@
if (unlikely(!(full | high)))
return -ENOTSUPP;
- /* Allocate */
- data = kmalloc(sizeof *data, GFP_KERNEL);
- if (unlikely(!data))
+ /* Allocate a single chunk, less management later on */
+ char *vlabuf =
[PATCH] Remove VLAIS usage from gadget code - alternate patch
From: Mark Charlebois charl...@gmail.com
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 alternate patch calculates offsets into the kmalloc-ed
memory buffer using macros. The previous patch required multiple kmalloc and
kfree calls.
Signed-off-by: Mark Charlebois charl...@gmail.com
Signed-off-by: Behan Webster beh...@converseincode.com
---
--- linux.orig/drivers/usb/gadget/f_fs.c
+++ linux/drivers/usb/gadget/f_fs.c
@@ -30,6 +30,21 @@
#define FUNCTIONFS_MAGIC 0xa647361 /* Chosen by a honest dice roll ;) */
+/* Variable Length Array Macros **/
+#define vla_struct(structname) size_t structname##__##next = 0
+#define vla_struct_size(structname) structname##__##next
+
+#define vla_item(structname, type, name, n) \
+ type * structname##_##name; \
+ size_t structname##_##name##__##offset = \
+ (structname##__##next + __alignof__(type) - 1) \
+ ~(__alignof__(type) - 1); \
+ size_t structname##_##name##__##sz = n * sizeof(type); \
+ structname##__##next = structname##_##name##__##offset + \
+ structname##_##name##__##sz;
+
+#define vla_ptr(ptr,structname,name) structname##_##name = \
+ (__typeof__(structname##_##name))ptr[structname##_##name##__##offset]
/* Debugging /
@@ -1909,30 +1924,38 @@
/* 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_struct(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));
+
+ char *vlabuf = kmalloc(vla_struct_size(d), GFP_KERNEL);
- d = kmalloc(sizeof *d, GFP_KERNEL);
- if (unlikely(!d)) {
+ 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 free */
+ stringtabs = d_stringtabs;
+ t = d_stringtab;
+ s = d_strings;
strings = s;
}
@@ -2208,16 +2231,15 @@
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;
+ vla_struct(d);
+ vla_item(d, struct ffs_ep, eps, ffs-eps_count);
+ vla_item(d, struct usb_descriptor_header *, fs_descs,
+ full ? ffs-fs_descs_count + 1 : 0);
+ vla_item(d, struct usb_descriptor_header *, hs_descs,
+ high ? ffs-hs_descs_count + 1 : 0);
+ vla_item(d, short, inums, ffs-interfaces_count);
+ vla_item(d, char, raw_descs,
+ high ? ffs-raw_descs_length : ffs-raw_fs_descs_length);
ENTER();
@@ -2225,21 +2247,30 @@
if (unlikely(!(full | high)))
return -ENOTSUPP;
- /* Allocate */
- data = kmalloc(sizeof *data, GFP_KERNEL);
- if (unlikely(!data))
+ /* Allocate a single chunk, less management later on */
+ char *vlabuf = kmalloc(vla_struct_size(d), GFP_KERNEL);
+ if (unlikely(!vlabuf))
return -ENOMEM;
+ /* Initialize each struct member pointer in the allocated memory */
+ vla_ptr(vlabuf, d, eps);
+
Re: [PATCH] Remove VLAIS usage from gadget code - alternate patch
On 09/24/13 13:56, charl...@gmail.com wrote: From: Mark Charlebois charl...@gmail.com 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 alternate patch calculates offsets into the kmalloc-ed memory buffer using macros. The previous patch required multiple kmalloc and kfree calls. Signed-off-by: Mark Charlebois charl...@gmail.com Signed-off-by: Behan Webster beh...@converseincode.com This one uses macros and carves up a single piece of memory from one kmalloc. Behan -- Behan Webster beh...@converseincode.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
