linux-kernel - Re: [RFC] x86: merge nmi

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: <7b9198260805171328u555eec17t3597f3378edbda88@mail.gmail.com>
Date:	Sat, 17 May 2008 21:28:18 +0100
From:	"Tom Spink" <tspink@...il.com>
To:	"Cyrill Gorcunov" <gorcunov@...il.com>
Cc:	"Ingo Molnar" <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	"Jiri Slaby" <jirislaby@...il.com>,
	"Andi Kleen" <andi@...stfloor.org>
Subject: Re: [RFC] x86: merge nmi_32-64 to nmi.c

2008/5/17 Cyrill Gorcunov <gorcunov@...il.com>:
> +/* a few helper functions */
> +#ifdef CONFIG_X86_64
> +
> +static inline unsigned int get_nmi_count(int cpu)
> +{
> +       return cpu_pda(cpu)->__nmi_count;
> +}
> +
> +static inline int mce_in_progress(void)
> +{
> +#ifdef CONFIG_X86_MCE
> +       return atomic_read(&mce_entry) > 0;
> +#endif
> +       return 0;
> +}
> +
> +static inline void __die_nmi(char *str, struct pt_regs *regs, int do_panic)
> +{
> +       die_nmi(str, regs, do_panic);
> +}
> +
> +#else /* !CONFIG_X86_64 */
> +
> +static inline unsigned int get_nmi_count(int cpu)
> +{
> +       return nmi_count(cpu);
> +}
> +
> +static inline int mce_in_progress(void)
> +{
> +       return 0;
> +}
> +
> +static inline void __die_nmi(char *str, struct pt_regs *regs, int do_panic)
> +{
> +       die_nmi(regs, str);
> +}
> +
> +#endif /* !CONFIG_X86_64 */

Hi,

I've always wondered if it's cleaner to define variants of functions
like this with the conditionals inside the function, as opposed to one
big conditional encapsulating all these functions.  IMO, it's cleaner
to define the function with conditionals to define it's particular
behaviour in the two different cases, because that way there is one
definition of the function with both different behaviours inside,
e.g.:

static inline unsigned int get_nmi_count(int cpu)
{
#ifdef CONFIG_X86_64
        return cpu_pda(cpu)->__nmi_count;
#else
        return nmi_count(cpu);
#endif
}

I know it introduces a lot of these conditionals, but at least there
is one place to look for the get_nmi_count function, instead of
searching for all variants of the function.

Just a thought!

-- 
Regards,
Tom Spink
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/