From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759896AbbCDSuu (ORCPT ); Wed, 4 Mar 2015 13:50:50 -0500 Received: from mail-wi0-f174.google.com ([209.85.212.174]:46719 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757809AbbCDSut (ORCPT ); Wed, 4 Mar 2015 13:50:49 -0500 Date: Wed, 4 Mar 2015 19:50:44 +0100 From: Ingo Molnar To: Darshana Padmadas Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, x86@kernel.org, josh@joshtriplett.org, Borislav Petkov Subject: Re: [PATCH 01/16] arch: x86: boot: Make function static and add function prototype Message-ID: <20150304185043.GA5110@gmail.com> References: <40cee6529458e725e48fddac329d9a9da0de67c3.1425473428.git.darshanapadmadas@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <40cee6529458e725e48fddac329d9a9da0de67c3.1425473428.git.darshanapadmadas@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Darshana Padmadas wrote: > compressed/eboot.c defines an internal function > setup_graphics(struct boot_params *boot_params). No > other file refers to this function with the same > parameters so make this function static. > > This eliminates the following warning: > > arch/x86/boot/compressed/eboot.c:1004:6: warning: no previous prototype for ‘setup_graphics’ [-Wmissing-prototypes] > > Also make_boot_params is declared only in this file and is used > in some assembly. So add a prototype for this function. > > This eliminates the following warning: > > arch/x86/boot/compressed/eboot.c:1042:21: warning: no previous prototype for ‘make_boot_params’ [-Wmissing-prototypes] > > Signed-off-by: Darshana Padmadas > --- > arch/x86/boot/compressed/eboot.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c > index 92b9a5f..cb4ebab 100644 > --- a/arch/x86/boot/compressed/eboot.c > +++ b/arch/x86/boot/compressed/eboot.c > @@ -1001,7 +1001,7 @@ free_handle: > return status; > } > > -void setup_graphics(struct boot_params *boot_params) > +static void setup_graphics(struct boot_params *boot_params) This category of change is good: making needlessly global functions static is absolutely useful even if the kernel doesn't use the -Wmissing-prototypes warning, because this allows the compiler to potentially inline the function and thus reduce the size of the kernel. (even if it's not inlined, it's a small reduction in kernel image size.) > { > efi_guid_t graphics_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; > struct screen_info *si; > @@ -1039,6 +1039,9 @@ void setup_graphics(struct boot_params *boot_params) > * The caller is responsible for filling out ->code32_start in the > * returned boot_params. > */ > + > +struct boot_params *make_boot_params(struct efi_config *c); > + > struct boot_params *make_boot_params(struct efi_config *c) This category of change is not so useful in that form: look at the duplicated line in the .c file, it looks weird and not very useful at first glance already. The way this is typically solved in user-space projects that use -Wall -Werror (such as tools/perf/) is to stick the external prototypes into a proper header file. If a function is global because it's only interfacing with assembly code then put that into a comment before the prototype lines as well, in the header file. In this case the extra prototypes are also useful: they keep people modifying those functions informed and careful, as changing the function parameters signature could break assembly code without creating any obvious build time errors or warnings. Thanks, Ingo