Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-16 Thread Takashi Iwai 
At Sat, 13 Jan 2007 07:37:59 +0100,
Oliver Neukum wrote:
> 
> Am Freitag, 12. Januar 2007 18:42 schrieb Takashi Iwai:
> > At Fri, 12 Jan 2007 14:49:57 +0100,
> > Oliver Neukum wrote:
> > > 
> > > + } else {
> > > +  if (idx < snd_ecards_limit) {
> > > + if (snd_cards_lock & (1 << idx))
> > > + err = -EBUSY;   /* invalid */
> > > + } else if (idx < SNDRV_CARDS)
> > > + snd_ecards_limit = idx + 1; /* increase the 
> > > limit */
> > > + else
> > > + err = -ENODEV;
> > 
> > The indent looks strange in the above three lines.
> > Also, for me it's not much better than before... :)
> > (all if's are comparisons of idx with other values.)
> 
> Hi,
> 
> OK, how about this one? The original indentation makes the control
> flow very hard to follow.
> 
>   Regards
>   Oliver
> 
> Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>

Thanks, now applied to ALSA tree.


Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-16 Thread Takashi Iwai 
At Sat, 13 Jan 2007 07:37:59 +0100,
Oliver Neukum wrote:
 
 Am Freitag, 12. Januar 2007 18:42 schrieb Takashi Iwai:
  At Fri, 12 Jan 2007 14:49:57 +0100,
  Oliver Neukum wrote:
   
   + } else {
   +  if (idx  snd_ecards_limit) {
   + if (snd_cards_lock  (1  idx))
   + err = -EBUSY;   /* invalid */
   + } else if (idx  SNDRV_CARDS)
   + snd_ecards_limit = idx + 1; /* increase the 
   limit */
   + else
   + err = -ENODEV;
  
  The indent looks strange in the above three lines.
  Also, for me it's not much better than before... :)
  (all if's are comparisons of idx with other values.)
 
 Hi,
 
 OK, how about this one? The original indentation makes the control
 flow very hard to follow.
 
   Regards
   Oliver
 
 Signed-off-by: Oliver Neukum [EMAIL PROTECTED]

Thanks, now applied to ALSA tree.


Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Oliver Neukum 
Am Freitag, 12. Januar 2007 18:42 schrieb Takashi Iwai:
> At Fri, 12 Jan 2007 14:49:57 +0100,
> Oliver Neukum wrote:
> > 
> > +   } else {
> > +if (idx < snd_ecards_limit) {
> > +   if (snd_cards_lock & (1 << idx))
> > +   err = -EBUSY;   /* invalid */
> > +   } else if (idx < SNDRV_CARDS)
> > +   snd_ecards_limit = idx + 1; /* increase the 
> > limit */
> > +   else
> > +   err = -ENODEV;
> 
> The indent looks strange in the above three lines.
> Also, for me it's not much better than before... :)
> (all if's are comparisons of idx with other values.)

Hi,

OK, how about this one? The original indentation makes the control
flow very hard to follow.

Regards
Oliver

Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
--

--- sound/core/init.c.alt   2007-01-12 14:26:47.0 +0100
+++ sound/core/init.c   2007-01-13 07:34:29.0 +0100
@@ -114,22 +114,28 @@
if (idx < 0) {
int idx2;
for (idx2 = 0; idx2 < SNDRV_CARDS; idx2++)
+   /* idx == -1 == 0x means: take any free slot */
if (~snd_cards_lock & idx & 1<= snd_ecards_limit)
snd_ecards_limit = idx + 1;
break;
}
-   } else if (idx < snd_ecards_limit) {
-   if (snd_cards_lock & (1 << idx))
-   err = -ENODEV;  /* invalid */
-   } else if (idx < SNDRV_CARDS)
-   snd_ecards_limit = idx + 1; /* increase the limit */
-   else
-   err = -ENODEV;
+   } else {
+if (idx < snd_ecards_limit) {
+   if (snd_cards_lock & (1 << idx))
+   err = -EBUSY;   /* invalid */
+   } else {
+   if (idx < SNDRV_CARDS)
+   snd_ecards_limit = idx + 1; /* increase the 
limit */
+   else
+   err = -ENODEV;
+   }
+   }
if (idx < 0 || err < 0) {
mutex_unlock(_card_mutex);
-   snd_printk(KERN_ERR "cannot find the slot for index %d (range 
0-%i)\n", idx, snd_ecards_limit - 1);
+   snd_printk(KERN_ERR "cannot find the slot for index %d (range 
0-%i), error: %d\n",
+idx, snd_ecards_limit - 1, err);
goto __error;
}
snd_cards_lock |= 1 << idx; /* lock it */

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Takashi Iwai 
At Fri, 12 Jan 2007 14:49:57 +0100,
Oliver Neukum wrote:
> 
> + } else {
> +  if (idx < snd_ecards_limit) {
> + if (snd_cards_lock & (1 << idx))
> + err = -EBUSY;   /* invalid */
> + } else if (idx < SNDRV_CARDS)
> + snd_ecards_limit = idx + 1; /* increase the 
> limit */
> + else
> + err = -ENODEV;

The indent looks strange in the above three lines.
Also, for me it's not much better than before... :)
(all if's are comparisons of idx with other values.)


Takashi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Oliver Neukum 
Hi,

please accept this patch, which makes the control flow clear with
indentation, adds some comments and improves error reporting.

Regards
Oliver

Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>

--
--- a/sound/core/init.c 2007-01-12 14:26:47.0 +0100
+++ b/sound/core/init.c 2007-01-12 14:46:13.0 +0100
@@ -114,22 +114,26 @@
if (idx < 0) {
int idx2;
for (idx2 = 0; idx2 < SNDRV_CARDS; idx2++)
+   /* idx == -1 == 0x means: take any free slot */
if (~snd_cards_lock & idx & 1<= snd_ecards_limit)
snd_ecards_limit = idx + 1;
break;
}
-   } else if (idx < snd_ecards_limit) {
-   if (snd_cards_lock & (1 << idx))
-   err = -ENODEV;  /* invalid */
-   } else if (idx < SNDRV_CARDS)
-   snd_ecards_limit = idx + 1; /* increase the limit */
-   else
-   err = -ENODEV;
+   } else {
+if (idx < snd_ecards_limit) {
+   if (snd_cards_lock & (1 << idx))
+   err = -EBUSY;   /* invalid */
+   } else if (idx < SNDRV_CARDS)
+   snd_ecards_limit = idx + 1; /* increase the 
limit */
+   else
+   err = -ENODEV;
+   }
if (idx < 0 || err < 0) {
mutex_unlock(_card_mutex);
-   snd_printk(KERN_ERR "cannot find the slot for index %d (range 
0-%i)\n", idx, snd_ecards_limit - 1);
+   snd_printk(KERN_ERR "cannot find the slot for index %d (range 
0-%i), error: %d\n",
+idx, snd_ecards_limit - 1, err);
goto __error;
}
snd_cards_lock |= 1 << idx; /* lock it */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Oliver Neukum 
Hi,

please accept this patch, which makes the control flow clear with
indentation, adds some comments and improves error reporting.

Regards
Oliver

Signed-off-by: Oliver Neukum [EMAIL PROTECTED]

--
--- a/sound/core/init.c 2007-01-12 14:26:47.0 +0100
+++ b/sound/core/init.c 2007-01-12 14:46:13.0 +0100
@@ -114,22 +114,26 @@
if (idx  0) {
int idx2;
for (idx2 = 0; idx2  SNDRV_CARDS; idx2++)
+   /* idx == -1 == 0x means: take any free slot */
if (~snd_cards_lock  idx  1idx2) {
idx = idx2;
if (idx = snd_ecards_limit)
snd_ecards_limit = idx + 1;
break;
}
-   } else if (idx  snd_ecards_limit) {
-   if (snd_cards_lock  (1  idx))
-   err = -ENODEV;  /* invalid */
-   } else if (idx  SNDRV_CARDS)
-   snd_ecards_limit = idx + 1; /* increase the limit */
-   else
-   err = -ENODEV;
+   } else {
+if (idx  snd_ecards_limit) {
+   if (snd_cards_lock  (1  idx))
+   err = -EBUSY;   /* invalid */
+   } else if (idx  SNDRV_CARDS)
+   snd_ecards_limit = idx + 1; /* increase the 
limit */
+   else
+   err = -ENODEV;
+   }
if (idx  0 || err  0) {
mutex_unlock(snd_card_mutex);
-   snd_printk(KERN_ERR cannot find the slot for index %d (range 
0-%i)\n, idx, snd_ecards_limit - 1);
+   snd_printk(KERN_ERR cannot find the slot for index %d (range 
0-%i), error: %d\n,
+idx, snd_ecards_limit - 1, err);
goto __error;
}
snd_cards_lock |= 1  idx; /* lock it */
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Takashi Iwai 
At Fri, 12 Jan 2007 14:49:57 +0100,
Oliver Neukum wrote:
 
 + } else {
 +  if (idx  snd_ecards_limit) {
 + if (snd_cards_lock  (1  idx))
 + err = -EBUSY;   /* invalid */
 + } else if (idx  SNDRV_CARDS)
 + snd_ecards_limit = idx + 1; /* increase the 
 limit */
 + else
 + err = -ENODEV;

The indent looks strange in the above three lines.
Also, for me it's not much better than before... :)
(all if's are comparisons of idx with other values.)


Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch]cleanup and error reporting for sound/core/init.c

2007-01-12 Thread Oliver Neukum 
Am Freitag, 12. Januar 2007 18:42 schrieb Takashi Iwai:
 At Fri, 12 Jan 2007 14:49:57 +0100,
 Oliver Neukum wrote:
  
  +   } else {
  +if (idx  snd_ecards_limit) {
  +   if (snd_cards_lock  (1  idx))
  +   err = -EBUSY;   /* invalid */
  +   } else if (idx  SNDRV_CARDS)
  +   snd_ecards_limit = idx + 1; /* increase the 
  limit */
  +   else
  +   err = -ENODEV;
 
 The indent looks strange in the above three lines.
 Also, for me it's not much better than before... :)
 (all if's are comparisons of idx with other values.)

Hi,

OK, how about this one? The original indentation makes the control
flow very hard to follow.

Regards
Oliver

Signed-off-by: Oliver Neukum [EMAIL PROTECTED]
--

--- sound/core/init.c.alt   2007-01-12 14:26:47.0 +0100
+++ sound/core/init.c   2007-01-13 07:34:29.0 +0100
@@ -114,22 +114,28 @@
if (idx  0) {
int idx2;
for (idx2 = 0; idx2  SNDRV_CARDS; idx2++)
+   /* idx == -1 == 0x means: take any free slot */
if (~snd_cards_lock  idx  1idx2) {
idx = idx2;
if (idx = snd_ecards_limit)
snd_ecards_limit = idx + 1;
break;
}
-   } else if (idx  snd_ecards_limit) {
-   if (snd_cards_lock  (1  idx))
-   err = -ENODEV;  /* invalid */
-   } else if (idx  SNDRV_CARDS)
-   snd_ecards_limit = idx + 1; /* increase the limit */
-   else
-   err = -ENODEV;
+   } else {
+if (idx  snd_ecards_limit) {
+   if (snd_cards_lock  (1  idx))
+   err = -EBUSY;   /* invalid */
+   } else {
+   if (idx  SNDRV_CARDS)
+   snd_ecards_limit = idx + 1; /* increase the 
limit */
+   else
+   err = -ENODEV;
+   }
+   }
if (idx  0 || err  0) {
mutex_unlock(snd_card_mutex);
-   snd_printk(KERN_ERR cannot find the slot for index %d (range 
0-%i)\n, idx, snd_ecards_limit - 1);
+   snd_printk(KERN_ERR cannot find the slot for index %d (range 
0-%i), error: %d\n,
+idx, snd_ecards_limit - 1, err);
goto __error;
}
snd_cards_lock |= 1  idx; /* lock it */

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/