lkml.org 
[lkml]   [2005]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] libfs: add simple attribute files
Date
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.

Arnd <><
-
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-05-19 10:49    [W:0.160 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site