[RFC/PATCH] Per-device parameter support (2/16)

[RFC/PATCH] Per-device parameter support (2/16)

[Tejun Heo]

 dp_02_param_array_bug.diff

 This is the 2nd patch of 16 patches for devparam.

 This patches fixes param_array_set() to not use arr->max as nump
argument of param_array.  If arr->max is used as nump and the
configuration variable is exported writeable in the syfs, the size of
the array will be limited by the smallest number of elements
specified.  One side effect is that as the actual number of elements
is not recorded anymore when nump is NULL, all elements should be
printed when referencing the corresponding sysfs node.  I don't think
that will cause any problem.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-devparam-export/kernel/params.c
===================================================================
--- linux-devparam-export.orig/kernel/params.c	2004-10-22 17:13:33.000000000 +0900
+++ linux-devparam-export/kernel/params.c	2004-10-23 11:09:28.000000000 +0900
@@ -302,9 +302,10 @@ int param_array(const char *name,
 int param_array_set(const char *val, struct kernel_param *kp)
 {
 	struct kparam_array *arr = kp->arg;
+	unsigned int t;
 
 	return param_array(kp->name, val, 1, arr->max, arr->elem,
-			   arr->elemsize, arr->set, arr->num ?: &arr->max);
+			   arr->elemsize, arr->set, arr->num ?: &t);
 }
 
 int param_array_get(char *buffer, struct kernel_param *kp)

Re: [RFC/PATCH] Per-device parameter support (2/16)

[Rusty Russell]

On Sat, 2004-10-23 at 13:24 +0900, Tejun Heo wrote:
>  dp_02_param_array_bug.diff
> 
>  This is the 2nd patch of 16 patches for devparam.
> 
>  This patches fixes param_array_set() to not use arr->max as nump
> argument of param_array.  If arr->max is used as nump and the
> configuration variable is exported writeable in the syfs, the size of
> the array will be limited by the smallest number of elements
> specified.  One side effect is that as the actual number of elements
> is not recorded anymore when nump is NULL, all elements should be
> printed when referencing the corresponding sysfs node.  I don't think
> that will cause any problem.

I thought of this, but I prefer to see this fixed by another element in
the struct kernel_param which is used as "num" if nump is NULL.
Although this creates some bloat, it doesn't truncate as mine does, or
allow overflows and printing unset values as yours does.

(Printing unset values is usually OK, since before the new parameter
stuff, there was no way of telling how many elements had been set.  This
lead authors to use a magic value for "unset".  They no longer need to
do this, so we might see them start to rely on that).

Cheers,
Rusty.
-- 
A bad analogy is like a leaky screwdriver -- Richard Braakman

Re: [RFC/PATCH] Per-device parameter support (2/16)

[Tejun Heo]

Hello, Rusty.

On Mon, Oct 25, 2004 at 02:28:59PM +1000, Rusty Russell wrote:
> On Sat, 2004-10-23 at 13:24 +0900, Tejun Heo wrote:
> >  dp_02_param_array_bug.diff
> > 
> >  This is the 2nd patch of 16 patches for devparam.
> > 
> >  This patches fixes param_array_set() to not use arr->max as nump
> > argument of param_array.  If arr->max is used as nump and the
> > configuration variable is exported writeable in the syfs, the size of
> > the array will be limited by the smallest number of elements
> > specified.  One side effect is that as the actual number of elements
> > is not recorded anymore when nump is NULL, all elements should be
> > printed when referencing the corresponding sysfs node.  I don't think
> > that will cause any problem.
> 
> I thought of this, but I prefer to see this fixed by another element in
> the struct kernel_param which is used as "num" if nump is NULL.
> Although this creates some bloat, it doesn't truncate as mine does, or
> allow overflows and printing unset values as yours does.

 The problem is that, in devparam, a devparam_def and accordingly any
wrapping argument structure is shared by more than one users (e.g. a
dev paramset_def is used by all devices attached to the particular
driver).  And, making matters complicated, bus and class
paramset_def's can even be accessed concurrently.

 As we need to adjust pointers in wrapping structures to point to
specific fields in each allocated paramsets, wrapping arguments are
copied and fixed up everytime it's used (so the arg_copy_size and
arg_fixups fields in struct device_paramset_def).  i.e. wrapping
arguments are transient.  They can contain extra read-only information
to describe the parameter to the driver-model but cannot contain any
information the other way around.  Consequently, adding num field to
struct param_array wouldn't work for devparam.

> (Printing unset values is usually OK, since before the new parameter
> stuff, there was no way of telling how many elements had been set.  This
> lead authors to use a magic value for "unset".  They no longer need to
> do this, so we might see them start to rely on that).

 I think, for a user who's willing to make use of the number of set
elements, requiring to supply the number field is okay.

-- 
tejun