Messages in this thread | | | Date | Wed, 4 Mar 2015 19:50:44 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 01/16] arch: x86: boot: Make function static and add function prototype |
| |
* Darshana Padmadas <darshanapadmadas@gmail.com> 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 <darshanapadmadas@gmail.com> > --- > 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
| |