Re: [PATCH 10/16] usb/gadget: FunctionFS: Remove VLAIS usage from gadget code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



W dniu 07.11.2013 19:28, Mark Charlebois pisze:



On Sat, Nov 2, 2013 at 10:44 AM, Michal Nazarewicz <mina86@xxxxxxxxxx
<mailto:mina86@xxxxxxxxxx>> wrote:

    On Wed, Oct 23 2013, Andrzej Pietrasiewicz wrote:
     > From: Mark Charlebois <charlebm@xxxxxxxxx
    <mailto:charlebm@xxxxxxxxx>>
     >
     > 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 <charlebm@xxxxxxxxx
    <mailto:charlebm@xxxxxxxxx>>
     > Signed-off-by: Behan Webster <behanw@xxxxxxxxxxxxxxxxxx
    <mailto:behanw@xxxxxxxxxxxxxxxxxx>>
     >
     > [elimination of miexed declaration and code, checkpatch cleanup]
     > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx
    <mailto:andrzej.p@xxxxxxxxxxx>>
     > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx
    <mailto:kyungmin.park@xxxxxxxxxxx>>
     > ---
     >  drivers/usb/gadget/f_fs.c |  110
    +++++++++++++++++++++++++++++----------------
     >  1 files changed, 71 insertions(+), 39 deletions(-)
     >
     > 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
     > @@ -30,6 +30,21 @@
     >
     >  #define FUNCTIONFS_MAGIC     0xa647361 /* Chosen by a honest
    dice roll ;) */

    I think something more explicit with fewer automatically declared
    variables cloud be more readable:

     > +/* Variable Length Array Macros
    **********************************************/
     > +#define vla_group(groupname) size_t groupname##__##next = 0
     > +#define vla_group_size(groupname) groupname##__##next

    #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_item(groupname, type, name, n)
               \
             size_t groupname##_##name##__offset = ({
                 \
                     size_t align_mask = __alignof__(type) - 1;
                 \
                     size_t offset = (groupname##__next + align_mask) &
    align_mask; \
                     size_t size = (n) * sizeof(type);
                \
                     groupname##__next = offset + size;
                 \
                     offset;
             })


I agree. Your proposal is cleaner. I can resubmit it.



I have already done so in the v2 series.

There is an issue pointed by Sebastian, though.

AP

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux