orinoco driver question

From: Mariusz Kozlowski
Date: Fri Nov 03 2006 - 10:14:58 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/