orinoco driver question

[Mariusz Kozlowski <m.kozlowski@tuxland.pl>]

Hi there,

	Hope that's not a problem to ask some 'newbie' kernel coding stuff here. Here 
it goes:

There are those two orinoco ioctl's

- orinoco_ioctl_setport3
- orinoco_ioctl_getport3

Both take 'char *extra' as an argument to set/get 'priv->prefer_port3'. The 
argument value to orinoco_ioctl_setport3 can be either 0 (IEEE ad-hoc mode) 
or 1 (Lucent proprietary ad-hoc mode) the rest is -EINVAL. I don't get why 
there is a need for an extra 'int' variable and casts in the code. 
Using 'char *extra' seems to be fine there. To visualize what I mean here is 
the patch:

--- linux-2.6.19-rc4-orig/drivers/net/wireless/orinoco.c        2006-11-02 
23:52:39.000000000 +0100
+++ linux-2.6.19-rc4/drivers/net/wireless/orinoco.c     2006-11-03 
16:02:45.000000000 +0100
@@ -3658,14 +3658,13 @@ static int orinoco_ioctl_setport3(struct
                                  char *extra)
 {
        struct orinoco_private *priv = netdev_priv(dev);
-       int val = *( (int *) extra );
        int err = 0;
        unsigned long flags;
 
        if (orinoco_lock(priv, &flags) != 0)
                return -EBUSY;
 
-       switch (val) {
+       switch (*extra) {
        case 0: /* Try to do IEEE ad-hoc mode */
                if (! priv->has_ibss) {
                        err = -EINVAL;
@@ -3704,9 +3703,8 @@ static int orinoco_ioctl_getport3(struct
                                  char *extra)
 {
        struct orinoco_private *priv = netdev_priv(dev);
-       int *val = (int *) extra;
 
-       *val = priv->prefer_port3;
+       *extra = (char)priv->prefer_port3;
        return 0;
 }

I don't think this patch decreases code readability. This is just an example 
but if there are more functions like this doesn't removing 'redundant' (?) 
variables make the code better?

Regards,

	Mariusz Kozlowski
-
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/