linux-kernel - Re: [PATCH 08/46] kernel: MOve prototype declaration to header file include/linux/perf

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]

Date:	Thu, 27 Feb 2014 14:55:45 -0800
From:	josh@...htriplett.org
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Rashika Kheria <rashika.kheria@...il.com>,
	linux-kernel@...r.kernel.org, Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Subject: Re: [PATCH 08/46] kernel: MOve prototype declaration to header
 file include/linux/perf_event.h

On Thu, Feb 27, 2014 at 08:23:35PM +0100, Peter Zijlstra wrote:
> On Thu, Feb 27, 2014 at 07:51:50AM -0800, Josh Triplett wrote:
> > On Thu, Feb 27, 2014 at 12:54:14PM +0100, Peter Zijlstra wrote:
> > > On Thu, Feb 27, 2014 at 05:02:48PM +0530, Rashika Kheria wrote:
> > > > Add prototype declaration of function to header file
> > > > include/linux/perf_event.h because it is used by more than one file.
> > > > 
> > > > This eliminates the following warning in kernel/events/core.c:
> > > > kernel/events/core.c:3743:13: warning: no previous prototype for ‘arch_perf_update_userpage’ [-Wmissing-prototypes]
> > > 
> > > # git grep arch_perf_update_userpage
> > > arch/x86/kernel/cpu/perf_event.c:void arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
> > > kernel/events/core.c:void __weak arch_perf_update_userpage(struct perf_event_mmap_page *userpg, u64 now)
> > > kernel/events/core.c:   arch_perf_update_userpage(userpg, now);
> > > 
> > > 
> > > There's two definitions; one weak, and one usage site.
> > > 
> > > What gives?
> > 
> > There's no prototype for the function anywhere, so -Wmissing-prototypes
> > rightfully complains.  Adding the prototype to a header included in both
> > source files ensures that the function signatures must match, and
> > eliminates the warning.
> 
> Definitions don't require prior declarations. Only usage without prior
> definitions require them.
> 
> I still don't see a problem.

There's value in -Wmissing-prototypes; it's equivalent to the Sparse
check that ensures functions are marked as static when they're not used
outside the file they're defined in (though unlike the Sparse check it
only applies to functions, not data).  The goal isn't "make the warning
go away"; the goal of passing -Wmissing-prototypes is:

- Get rid of unused functions (many of which have been caught by this
  effort).
- Mark functions as static where possible (which enables the above and
  also improves code generation).
- Include headers that declare functions in the source files that define
  them, not just in the source files that use them.  That keeps the
  declaration and definition in sync (which has caught some real bugs as
  part of this effort).
- When no such header exists for a function used across multiple files,
  add the prototype to an appropriate header and use that.  Again, this
  has caught real bugs as part of this effort, when the definition and
  use were out of sync.  (For instance, disagreeing in return type.)
--
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/