linux-kernel - Re: [PATCH v1 02/10] x86, intel-mid: Remove "weak" from function declarations

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20141017014128.GA4513@psi-dev26.jf.intel.com>
Date:	Thu, 16 Oct 2014 18:41:29 -0700
From:	David Cohen <david.a.cohen@...ux.intel.com>
To:	sathyanarayanan kuppuswamy 
	<sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jason Wessel <jason.wessel@...driver.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Ingo Molnar <mingo@...hat.com>,
	John Stultz <john.stultz@...aro.org>,
	Eric Paris <eparis@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v1 02/10] x86, intel-mid: Remove "weak" from function
 declarations

Hi Bjorn and Sathya,

On Thu, Oct 16, 2014 at 05:42:11PM -0700, sathyanarayanan kuppuswamy wrote:
> Hi Bjorn,
> 
> On 10/15/2014 04:26 PM, Bjorn Helgaas wrote:
> >[+cc David, Kuppuswamy, x86; sorry, I botched my "stg mail" so you
> >weren't included the first time]
> >
> >On Wed, Oct 15, 2014 at 11:05 AM, Bjorn Helgaas <bhelgaas@...gle.com> wrote:
> >>For the following interfaces:
> >>
> >>   get_penwell_ops()
> >>   get_cloverview_ops()
> >>   get_tangier_ops()
> >>
> >>there is only one implementation, so they do not need to be marked "weak".
> >>
> >>Remove the "weak" attribute from their declarations.
> >>
> >>Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
> >>CC: David Cohen <david.a.cohen@...ux.intel.com>
> >>CC: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> >>CC: x86@...nel.org
> >>---
> >>  arch/x86/platform/intel-mid/intel_mid_weak_decls.h |    7 +++----
> >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>
> >>diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> >>index 46aa25c8ce06..3c1c3866d82b 100644
> >>--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> Please remove this file and move the contents to asm/intel-mid.h.

I partially agree :)

Historically, this file was created because we could not build all intel
mid variants at once. So we have to select only one during compilation
time, which was fixed already.

But we don't need to expose those functions outside intel-mid's
directory, which means asm/intel-mid.h isn't the best option IMHO.

If you want, I can send a small re-work instead: we get rid of this
header file completely and simplify a bit what is exposed by
asm/intel-mid.h. Or you can keep this patch and then I send the re-work
on top of it. Anyway I'm fine.

Br, David

> >>+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> >>@@ -10,10 +10,9 @@
> >>   */
> >>
> >>
> >>-/* __attribute__((weak)) makes these declarations overridable */
> >>  /* For every CPU addition a new get_<cpuname>_ops interface needs
> >>   * to be added.
> >>   */
> >>-extern void *get_penwell_ops(void) __attribute__((weak));
> >>-extern void *get_cloverview_ops(void) __attribute__((weak));
> >>-extern void *get_tangier_ops(void) __attribute__((weak));
> >>+extern void *get_penwell_ops(void);
> >>+extern void *get_cloverview_ops(void);
> >>+extern void *get_tangier_ops(void);
> >>
> 
> -- 
> Sathyanarayanan Kuppuswamy
> Android kernel developer
> 
--
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/