linux-kernel - Re: Fwd: [PATCH 7/9] ASoC: Blackfin: I2S CPU DAI driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <38521.1223537753@turing-police.cc.vt.edu>
Date:	Thu, 09 Oct 2008 03:35:53 -0400
From:	Valdis.Kletnieks@...edu
To:	John Kacur <jkacur@...il.com>
Cc:	Bryan Wu <cooloney@...nel.org>, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org, Cliff Cai <cliff.cai@...log.com>
Subject: Re: Fwd: [PATCH 7/9] ASoC: Blackfin: I2S CPU DAI driver

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.. :)


Content of type "application/pgp-signature" skipped