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    [W:0.023 / U:32.212 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site