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:
- Surely the default case is also an -EINVAL
- 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.. :)