git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Denton Liu <liu.denton@gmail.com>
Date
Oct 9, 2020, 23:00 UTC
Message-ID
<20201009230033.GA31120@generichostname>
In-Reply-To
<xmqqpn5rrvfg.fsf@gitster.c.googlers.com>
Hi Junio,
On Fri, Oct 09, 2020 at 01:33:39PM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > The argument for including it is less clear to me. You say below:
> >
> >> [...]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.
> >
> > but to my recollection that has not been a big problem. And it's one
> > that's usually easily caught by the compiler. A missing "extern" on a
> > variable will usually get you a multiple-definition warning at
> > link-time (if you manage to also omit the actual definition you won't
> > see that, though "make sparse" will warn that your variable ought to be
> > static).
> 
> Not really, that is where the "common" extension comes in, to help
> us with it hurt others without it, unknowingly X-<.
I'm not really sure what you mean by the "common" extension. 
Show 23 quoted lines
>         $ cat >a.c <<\EOF
>         #include <stdio.h>
>         #include "c.h"
> 
>         int common = 47;
> 
>         int main(int ac, char **av)
>         {
>             printf("%d\n", common + other);
>             return 0;
>         }
>         EOF
>         $ cat >b.c <<\EOF
>         #include "c.h"
> 
>         int other = 22;
>         EOF
>         $ cat >c.h <<\EOF
>         int common;
>         int other;
>         EOF
>         $ gcc -Wall -o c a.c b.c; ./c
>         59

On gcc 10.2.0, it errors out successfully. Although on clang 10.0.1, it compiles successfully and produces "69". That being said, I think extern variables are relatively rare in our codebase and, when it happens, they usually come as part of lists of other extern variables so a developer who's mimicking the surrounding code would be able to copy it successfully. Otherwise, the decl usually pops out in header files as it is quite unusual.

Show 6 quoted lines
> And I have a strong preference, after thinking about it, to have
> "extern" in front in the declarations.  It gives another clue for
> patterns I feed to "git grep" to latch onto, and help my eyes to
> scan and tell decls and defns apart in the output.  The benefit
> alone is worth the extra 7 columns in front spent, which you call
> "clutter".

To be honest, I do not have any preference between having the explicit extern or not. I do have a strong preference, however, for having a codebase that's consistently written. When I was doing the refactor, I wouldn't have minded introducing extern everywhere although that wasn't suggested as an alternative.

I agree that these are all benefits of declaring functions explicitly as extern. However, I don't think they're worth the cost of either another huge rewrite or an inconsistent codebase.

Show 15 quoted lines
> > IMHO the real problem here is that C's syntax for returning a function
> > pointer is so horrendous. How about this (on top of your earlier patch
> > to drop the extern from that declaration)?
> 
> In general, I like a typedef for callback function that shortens the
> decl of a function that takes such a callback, so I think
> 
> > +void set_error_routine(report_fn routine);
> > +void set_warn_routine(report_fn routine);
> > +report_fn get_error_routine(void);
> > +report_fn get_warn_routine(void);
> 
> these are good, but they are better with "extern" in front in a
> header file to make it clear they are declarations and not
> definitions when they appear in "git grep" output.
I agree that this looks a lot better, with or without the extern.
Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 15 in “clean up extern decl of functions”
  1. clean up extern decl of functionsJunio C Hamano, Oct 8, 2020
  2. Denton LiuOct 9, 2020
  3. Junio C HamanoOct 9, 2020
  4. Junio C HamanoOct 9, 2020
  5. [RFC] CodingGuidelines: mark external declarations with "extern"Junio C Hamano, Oct 9, 2020
  6. Jeff KingOct 9, 2020
  7. Junio C HamanoOct 9, 2020
  8. Denton LiuOct 9, 2020
  9. Junio C HamanoOct 9, 2020
  10. Đoàn Trần Công DanhOct 10, 2020
  11. Jeff KingOct 15, 2020
  12. Junio C HamanoOct 15, 2020
  13. Jeff KingOct 15, 2020
  14. 1/2 usage: define a type for a reporting functionJeff King, Oct 15, 2020
  15. 2/2 config.mak.dev: build with -fno-commonJeff King, Oct 15, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.