Subject: [RFC] CodingGuidelines: mark external declarations with "extern"

From: Junio C Hamano <[email-masked]>
Date: Fri, 09 Oct 2020 12:27:28 -0700

The document recommends not to write "extern" in front of a function
declaration, with justification that a decl by default is extern.

While it is true that by default decls are extern unless marked with
"static", it does not justify the choice.  It only justifies why we
could omit it if we wanted to, but does not say anything about the
reason why we would want to omit it in the first place.

Recently we saw that past mechanical conversion attempts kept a few
function decls with "extern" in front.  It seems that it was left
because the mechanical conversion tried not to touch external
declaration of pointers to functions, but the pattern used was
faulty and excluded functions that return pointers to functions.

For example, a pointer to a function may look like this:

    extern void (*default_frotz_handler)(int frotz);

We must not omit "extern" from this decl, which says "There exists a
variable whose name is default_frotz_handler, which points at a
function that takes an integer and returns nothing."  It is not a
function declaration and if written without "extern" in front,
requires the "common" extension to be correctly built.

But a function that returns a pointer to a function looks similar:

    extern void (*get_error_routine(void))(const char *message, ...);

which says "There is a get_error_routine() function that takes no
parameters, and it returns a pointer to a function that takes these
parameters and returns nothing".

The current rule tells us not to write "extern" in front, but the
earlier mechanical conversion missed it.  People when writing would
also miss it unless they are careful.

Instead of forcing contributors to spend time on on thinking if they
should or should not write "extern" in front of each of their decl
when they write new code, tell them that our external declarations
always say "extern" in front.  By doing so, we would also prevent a
mistake of not writing "extern" when we need to (i.e. decls of data
items, that are not functions) when less experienced developers try
to mimic how the existing surrounding declarations are written.

Signed-off-by: Junio C Hamano <[email-masked]>
---
 Documentation/CodingGuidelines | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 45465bc0c9..eafe41bec8 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -433,10 +433,12 @@ For C programs:
  - Use Git's gettext wrappers to make the user interface
    translatable. See "Marking strings for translation" in po/README.
 
- - Variables and functions local to a given source file should be marked
-   with "static". Variables that are visible to other source files
-   must be declared with "extern" in header files. However, function
-   declarations should not use "extern", as that is already the default.
+ - Variables and functions local to a given source file should be
+   marked with "static". Variables that are visible to other source
+   files must be declared with "extern" in header files.  External
+   function declarations should also use "extern", while external
+   function definition should not, to make it easier to visually tell
+   them apart.
 
  - You can launch gdb around your program using the shorthand GIT_DEBUGGER.
    Run `GIT_DEBUGGER=1 ./bin-wrappers/git foo` to simply use gdb as is, or
-- 
2.29.0-rc1-92-g713508e020