lkml.org 
[lkml]   [2008]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: Fwd: [PATCH 7/9] ASoC: Blackfin: I2S CPU DAI driver
From
Date
On Thu, 09 Oct 2008 08:58:08 +0200, John Kacur said:

> My eyes fell upon this switch statement, probably I have similar
> criticisms as to what has already been said, but:
> 1. Surely the default case is also an -EINVAL
> 2. Why not let all the EINVALS fall through, it will shorten up the
> code, and IMO make it more readable, something like this?
> + switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBM_CFM: /* Passing Case */
> + break;
> + case SND_SOC_DAIFMT_CBS_CFS: /* Failing Cases */
> + case SND_SOC_DAIFMT_CBM_CFS:
> + case SND_SOC_DAIFMT_CBS_CFM:
> + ret = -EINVAL;
> + break;
> + default:
> + printk(KERN_INFO "Unknown SND_SOC_DAIFMT kind\n");
> + ret = -EINVAL;
> + break;
> + }

Even shorter, but puts the default in a non-standard place:

+ switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+ case SND_SOC_DAIFMT_CBM_CFM: /* Passing Case */
+ break;
+ default: /* So much Fail we should say something */
+ printk(KERN_INFO "Unknown SND_SOC_DAIFMT kind\n");
+ case SND_SOC_DAIFMT_CBS_CFS: /* Failing Cases */
+ case SND_SOC_DAIFMT_CBM_CFS:
+ case SND_SOC_DAIFMT_CBS_CFM:
+ ret = -EINVAL;
+ break;
+ }
(I see a checkpatch update to check where default: is, coming in 3.. 2.. 1.. :)

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2008-10-09 09:39    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site