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    [from the cache]
©2003-2011 Jasper Spaans