Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 04:59:28PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Jul 10, 2013 at 04:00:57PM -0700, Guenter Roeck wrote:
> > > + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
> >
> > What happens here if grp->bin_attrs is NULL ? Doesn't that cause a NULL
> > reference when accessing *bin_attr ?
> >
> > [ The problem didn't exist before since attr was supposed to be non-NULL.
> > It may now be a problem if only bin_attr is provided, ie if there are
> > only binary attribute files.
> > Or maybe I am just tired and there is no problem... ]
>
> I thought I tested this patch out, but I think you are right, let me go
> do a rebuild and test it again...
You were right, good catch, as-is, the kernel dies at boot, I've fixed
this and will resend the series, thanks so much for the review.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 04:00:57PM -0700, Guenter Roeck wrote:
> On Wed, Jul 10, 2013 at 01:05:13PM -0700, Greg Kroah-Hartman wrote:
> > @@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd,
> > struct kobject *kobj,
> > }
> > if (error)
> > remove_files(dir_sd, kobj, grp);
>
> You might want to abort here if there was an error.
Good point, I'll update it.
> > +
> > + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
>
> What happens here if grp->bin_attrs is NULL ? Doesn't that cause a NULL
> reference when accessing *bin_attr ?
>
> [ The problem didn't exist before since attr was supposed to be non-NULL.
> It may now be a problem if only bin_attr is provided, ie if there are
> only binary attribute files.
> Or maybe I am just tired and there is no problem... ]
I thought I tested this patch out, but I think you are right, let me go
do a rebuild and test it again...
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 01:05:13PM -0700, Greg Kroah-Hartman wrote:
> groups should be able to support binary attributes, just like it
> supports "normal" attributes. This lets us only handle one type of
> structure, groups, throughout the driver core and subsystems, making
> binary attributes a "full fledged" part of the driver model, and not
> something just "tacked on".
>
> Reported-by: Oliver Schinagl
> Signed-off-by: Greg Kroah-Hartman
> ---
> fs/sysfs/group.c | 18 --
> include/linux/sysfs.h | 4 ++--
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index aec3d5c..d8d8a8f 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -20,16 +20,19 @@ static void remove_files(struct sysfs_dirent *dir_sd,
> struct kobject *kobj,
>const struct attribute_group *grp)
> {
> struct attribute *const* attr;
> - int i;
> + struct bin_attribute *const* bin_attr;
checkpatch doesn't like the above and similar declarations below.
Yes, I know, nitpick, just saying ;).
>
> - for (i = 0, attr = grp->attrs; *attr; i++, attr++)
> + for (attr = grp->attrs; *attr; attr++)
> sysfs_hash_and_remove(dir_sd, NULL, (*attr)->name);
> + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++)
> + sysfs_remove_bin_file(kobj, *bin_attr);
> }
>
> static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
> const struct attribute_group *grp, int update)
> {
> struct attribute *const* attr;
> + struct bin_attribute *const* bin_attr;
> int error = 0, i;
>
> for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
> @@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd,
> struct kobject *kobj,
> }
> if (error)
> remove_files(dir_sd, kobj, grp);
You might want to abort here if there was an error.
> +
> + for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
What happens here if grp->bin_attrs is NULL ? Doesn't that cause a NULL
reference when accessing *bin_attr ?
[ The problem didn't exist before since attr was supposed to be non-NULL.
It may now be a problem if only bin_attr is provided, ie if there are
only binary attribute files.
Or maybe I am just tired and there is no problem... ]
> + if (update)
> + sysfs_remove_bin_file(kobj, *bin_attr);
> + error = sysfs_create_bin_file(kobj, *bin_attr);
> + if (error)
> + break;
> + }
> + if (error)
> + remove_files(dir_sd, kobj, grp);
> +
> return error;
> }
>
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index d50a96b..2c3b6a3 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -21,6 +21,7 @@
>
> struct kobject;
> struct module;
> +struct bin_attribute;
> enum kobj_ns_type;
>
> struct attribute {
> @@ -59,10 +60,9 @@ struct attribute_group {
> umode_t (*is_visible)(struct kobject *,
> struct attribute *, int);
> struct attribute**attrs;
> + struct bin_attribute**bin_attrs;
> };
>
> -
> -
> /**
> * Use these macros to make defining attributes easier. See
> include/linux/device.h
> * for examples..
> --
> 1.8.3.rc0.20.gb99dd2e
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] sysfs: add support for binary attributes in groups
groups should be able to support binary attributes, just like it
supports "normal" attributes. This lets us only handle one type of
structure, groups, throughout the driver core and subsystems, making
binary attributes a "full fledged" part of the driver model, and not
something just "tacked on".
Reported-by: Oliver Schinagl
Signed-off-by: Greg Kroah-Hartman
---
fs/sysfs/group.c | 18 --
include/linux/sysfs.h | 4 ++--
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index aec3d5c..d8d8a8f 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -20,16 +20,19 @@ static void remove_files(struct sysfs_dirent *dir_sd,
struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int i;
+ struct bin_attribute *const* bin_attr;
- for (i = 0, attr = grp->attrs; *attr; i++, attr++)
+ for (attr = grp->attrs; *attr; attr++)
sysfs_hash_and_remove(dir_sd, NULL, (*attr)->name);
+ for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++)
+ sysfs_remove_bin_file(kobj, *bin_attr);
}
static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp, int update)
{
struct attribute *const* attr;
+ struct bin_attribute *const* bin_attr;
int error = 0, i;
for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
@@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd, struct
kobject *kobj,
}
if (error)
remove_files(dir_sd, kobj, grp);
+
+ for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
+ if (update)
+ sysfs_remove_bin_file(kobj, *bin_attr);
+ error = sysfs_create_bin_file(kobj, *bin_attr);
+ if (error)
+ break;
+ }
+ if (error)
+ remove_files(dir_sd, kobj, grp);
+
return error;
}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index d50a96b..2c3b6a3 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -21,6 +21,7 @@
struct kobject;
struct module;
+struct bin_attribute;
enum kobj_ns_type;
struct attribute {
@@ -59,10 +60,9 @@ struct attribute_group {
umode_t (*is_visible)(struct kobject *,
struct attribute *, int);
struct attribute**attrs;
+ struct bin_attribute**bin_attrs;
};
-
-
/**
* Use these macros to make defining attributes easier. See
include/linux/device.h
* for examples..
--
1.8.3.rc0.20.gb99dd2e
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/6] sysfs: add support for binary attributes in groups
groups should be able to support binary attributes, just like it
supports normal attributes. This lets us only handle one type of
structure, groups, throughout the driver core and subsystems, making
binary attributes a full fledged part of the driver model, and not
something just tacked on.
Reported-by: Oliver Schinagl oliver+l...@schinagl.nl
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
fs/sysfs/group.c | 18 --
include/linux/sysfs.h | 4 ++--
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index aec3d5c..d8d8a8f 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -20,16 +20,19 @@ static void remove_files(struct sysfs_dirent *dir_sd,
struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int i;
+ struct bin_attribute *const* bin_attr;
- for (i = 0, attr = grp-attrs; *attr; i++, attr++)
+ for (attr = grp-attrs; *attr; attr++)
sysfs_hash_and_remove(dir_sd, NULL, (*attr)-name);
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++)
+ sysfs_remove_bin_file(kobj, *bin_attr);
}
static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp, int update)
{
struct attribute *const* attr;
+ struct bin_attribute *const* bin_attr;
int error = 0, i;
for (i = 0, attr = grp-attrs; *attr !error; i++, attr++) {
@@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd, struct
kobject *kobj,
}
if (error)
remove_files(dir_sd, kobj, grp);
+
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++) {
+ if (update)
+ sysfs_remove_bin_file(kobj, *bin_attr);
+ error = sysfs_create_bin_file(kobj, *bin_attr);
+ if (error)
+ break;
+ }
+ if (error)
+ remove_files(dir_sd, kobj, grp);
+
return error;
}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index d50a96b..2c3b6a3 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -21,6 +21,7 @@
struct kobject;
struct module;
+struct bin_attribute;
enum kobj_ns_type;
struct attribute {
@@ -59,10 +60,9 @@ struct attribute_group {
umode_t (*is_visible)(struct kobject *,
struct attribute *, int);
struct attribute**attrs;
+ struct bin_attribute**bin_attrs;
};
-
-
/**
* Use these macros to make defining attributes easier. See
include/linux/device.h
* for examples..
--
1.8.3.rc0.20.gb99dd2e
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 01:05:13PM -0700, Greg Kroah-Hartman wrote:
groups should be able to support binary attributes, just like it
supports normal attributes. This lets us only handle one type of
structure, groups, throughout the driver core and subsystems, making
binary attributes a full fledged part of the driver model, and not
something just tacked on.
Reported-by: Oliver Schinagl oliver+l...@schinagl.nl
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
fs/sysfs/group.c | 18 --
include/linux/sysfs.h | 4 ++--
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index aec3d5c..d8d8a8f 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -20,16 +20,19 @@ static void remove_files(struct sysfs_dirent *dir_sd,
struct kobject *kobj,
const struct attribute_group *grp)
{
struct attribute *const* attr;
- int i;
+ struct bin_attribute *const* bin_attr;
checkpatch doesn't like the above and similar declarations below.
Yes, I know, nitpick, just saying ;).
- for (i = 0, attr = grp-attrs; *attr; i++, attr++)
+ for (attr = grp-attrs; *attr; attr++)
sysfs_hash_and_remove(dir_sd, NULL, (*attr)-name);
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++)
+ sysfs_remove_bin_file(kobj, *bin_attr);
}
static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
const struct attribute_group *grp, int update)
{
struct attribute *const* attr;
+ struct bin_attribute *const* bin_attr;
int error = 0, i;
for (i = 0, attr = grp-attrs; *attr !error; i++, attr++) {
@@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd,
struct kobject *kobj,
}
if (error)
remove_files(dir_sd, kobj, grp);
You might want to abort here if there was an error.
+
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++) {
What happens here if grp-bin_attrs is NULL ? Doesn't that cause a NULL
reference when accessing *bin_attr ?
[ The problem didn't exist before since attr was supposed to be non-NULL.
It may now be a problem if only bin_attr is provided, ie if there are
only binary attribute files.
Or maybe I am just tired and there is no problem... ]
+ if (update)
+ sysfs_remove_bin_file(kobj, *bin_attr);
+ error = sysfs_create_bin_file(kobj, *bin_attr);
+ if (error)
+ break;
+ }
+ if (error)
+ remove_files(dir_sd, kobj, grp);
+
return error;
}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index d50a96b..2c3b6a3 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -21,6 +21,7 @@
struct kobject;
struct module;
+struct bin_attribute;
enum kobj_ns_type;
struct attribute {
@@ -59,10 +60,9 @@ struct attribute_group {
umode_t (*is_visible)(struct kobject *,
struct attribute *, int);
struct attribute**attrs;
+ struct bin_attribute**bin_attrs;
};
-
-
/**
* Use these macros to make defining attributes easier. See
include/linux/device.h
* for examples..
--
1.8.3.rc0.20.gb99dd2e
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 04:00:57PM -0700, Guenter Roeck wrote:
On Wed, Jul 10, 2013 at 01:05:13PM -0700, Greg Kroah-Hartman wrote:
@@ -52,6 +55,17 @@ static int create_files(struct sysfs_dirent *dir_sd,
struct kobject *kobj,
}
if (error)
remove_files(dir_sd, kobj, grp);
You might want to abort here if there was an error.
Good point, I'll update it.
+
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++) {
What happens here if grp-bin_attrs is NULL ? Doesn't that cause a NULL
reference when accessing *bin_attr ?
[ The problem didn't exist before since attr was supposed to be non-NULL.
It may now be a problem if only bin_attr is provided, ie if there are
only binary attribute files.
Or maybe I am just tired and there is no problem... ]
I thought I tested this patch out, but I think you are right, let me go
do a rebuild and test it again...
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] sysfs: add support for binary attributes in groups
On Wed, Jul 10, 2013 at 04:59:28PM -0700, Greg Kroah-Hartman wrote:
On Wed, Jul 10, 2013 at 04:00:57PM -0700, Guenter Roeck wrote:
+ for (bin_attr = grp-bin_attrs; *bin_attr; bin_attr++) {
What happens here if grp-bin_attrs is NULL ? Doesn't that cause a NULL
reference when accessing *bin_attr ?
[ The problem didn't exist before since attr was supposed to be non-NULL.
It may now be a problem if only bin_attr is provided, ie if there are
only binary attribute files.
Or maybe I am just tired and there is no problem... ]
I thought I tested this patch out, but I think you are right, let me go
do a rebuild and test it again...
You were right, good catch, as-is, the kernel dies at boot, I've fixed
this and will resend the series, thanks so much for the review.
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
