[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200611042204.57560.mb@bu3sch.de>
Date: Sat, 4 Nov 2006 22:04:57 +0100
From: Michael Buesch <mb@...sch.de>
To: Mariusz Kozlowski <m.kozlowski@...land.pl>
Cc: linux-kernel@...r.kernel.org
Subject: Re: orinoco driver question
On Friday 03 November 2006 16:13, Mariusz Kozlowski wrote:
> 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?
This breaks on bigendian systems.
--
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists