lkml.org 
[lkml]   [2005]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] libfs: add simple attribute files
    On Thu, May 19, 2005 at 10:29:06AM +0200, Arnd Bergmann wrote:
    > On Middeweken 18 Mai 2005 22:24, Greg KH wrote:
    >
    > > Thanks for the patch. I've cleaned it up a bit (drop the spufs
    > > comments, changed the access check, and made the val be u64, and
    > > exported the symbols and cleaned up the debugfs portion) and added it to
    > > my tree. It should show up in the next -mm release. I've included the
    > > patch below so you can see my
    > > changes.
    >
    > Great, thanks for cleaning up those mistakes.
    >
    > I noticed one small problem with the change from 'long' to 'u64', in
    > that you did not change it in all places. In particular, using "%lu" to
    > print a u64 value will always do the wrong thing on big-endian 32 bit
    > platforms and maybe on some others.
    > Since 'u64' is '%llu' on most platforms but '%lu' on some 64 bit
    > platforms, I'd either do explicit cast to unsigned long long in
    > the printf or use unsigned long long throughout the code.
    >
    > > void foo_set(void *data, long val); and
    > ^^ u64
    > > long foo_get(void *data);
    > ^^ u64
    >
    > > +#define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt) \
    > > +static int __fops ## _open(struct inode *inode, struct file *file) \
    > > +{ \
    > > + __simple_attr_check_format(__fmt, 0ul); \
    > ^^^^ 0ull
    >
    > > + else /* first read */
    > > + size = scnprintf(attr->get_buf, sizeof(attr->get_buf),
    > > + attr->fmt, attr->get(attr->data));
    > ^^ (unsigned long long)
    >
    > > +DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs_u8_get, debugfs_u8_set, "%lu\n");
    > > +DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugfs_u16_get, debugfs_u16_set, "%lu\n");
    > > +DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugfs_u32_get, debugfs_u32_set, "%lu\n");
    > %llu ^^^^
    >
    > I also noticed that it is not possible to pass NULL operations to
    > DEFINE_SIMPLE_ATTRIBUTE() unless you change
    >
    > --- a/include/linux/fs.h 2005-05-19 10:17:53.000000000 +0200
    > +++ b/include/linux/fs.h 2005-05-19 10:14:57.000000000 +0200
    > @@ -1680,7 +1680,7 @@
    > static int __fops ## _open(struct inode *inode, struct file *file) \
    > { \
    > __simple_attr_check_format(__fmt, 0ul); \
    > - return simple_attr_open(inode, file, &__get, &__set, __fmt); \
    > + return simple_attr_open(inode, file, __get, __set, __fmt); \
    > } \
    > static struct file_operations __fops = { \
    > .owner = THIS_MODULE, \
    >
    > I'm currently away from my test machine, so I think it's easier if you
    > just update your patch yourself, but I could also send you an update
    > patch later if you prefer.

    Thanks for the updates, I've made them by hand to the patch, and will
    show up in the next -mm release.

    thanks again,

    greg k-h
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-06-10 06:39    [W:0.026 / U:59.360 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site