The most stupid C bug ever

I agree with the various other commenters on the blog post - that treating the wrapper function like an unacceptable amount of redirection is insane premature optimisation at expense of clarity.

Not to mention the wrapper can be inlined in C99, or would be automatically inlined with -O3 in gcc, not that it matters one iota in this case.

The other glaring bug would seem to be Windows' implementation for POSIX tmpfile() requiring admin access. But I guess that's not too surprising.

I've actually encountered this, but I can't say it stumped me at all.. my syntax highlighting made it immediately apparent what would have happened had I tried to build.

A coworker of mine also ran into a similar issue (not realizing something was commented out, thus odd behaviour) with a if 0 .... #endif comment. Vim highlights that correctly too but his emacs setup did not (no idea how stock it was).

"That has a performance impact on all platforms..." I mean, tmpfile() is about to make AT LEAST one system call, access the disk, and do a bunch of stuff he has no control over whatsoever, yet he's worried about one extra CALL instruction. Talk about premature optimization.. (and incompetence, if you will).

The comment from SteveB in the linked-to thread was interesting:

   Most editors i have used highlight the next line in green as a comment, 
   however i have come across one that took me over a year to solve, that is 
   remarkably similar.

        //why does this next line not compile??/
        x = 0;

    because the ??/ operator is a trigraph meaning \
    That was never been highlighted correctly any editor I have used.

Compiling with gcc's -Wall option, which implies -Wcomment, would spit out "warning: multi-line comment". Unless you have good reason to do otherwise, always use -Wall. I recommend -Wextra and -Werror as well.

"That has a performance impact on all platforms: the direct call to tmpfile() is now an indirect, which defeats optimization, and well, everything."

This couldn't be any more wrong. First of all, even if there were a performance impact, it would be meaningless compared to the time it takes to perform the system calls required for tmpfile().

Second, the compiler will inline the function call if it can. You can use a regular "inline" function and if you're not convinced it's good enough, then use __attribute__(force_inline) or equivalent. Even if the damn function is not an inline function, it will get inlined if it's in the same translation unit as where it is called (look at the asm output of gcc -O3). In the (not so far) future we'll be able to enjoy link time optimizations and don't have to care about translation units w.r.t. inlining optimizations.

I too am tired of paying the price for C's notoriously expensive function calls.

This is a good article because it shows the author's thought process. That means we can see what he did wrong and learn to think differently.

The first thing that sticks out as problematic is the assertion that a level of indirection around tmpfile is going to be slow. There are two ways we can determine if this is true; Try It And See, or read the output of the compiler to see what code it generates. Let's try the second one.

We'll make a file called slow.c with the contents:

    int f(void){
      return 42;
    }

    int g(void){
      return f();
    }

Then we can see what code gcc generates with the -S option:

    $ gcc -S slow.c

and looking at slow.s. Here's the important part of g (with some annotations removed):

    g:
    .LFB1:
	pushq	%rbp
	movq	%rsp, %rbp
	call	f
	popq	%rbp
	ret

So, we're calling f here. But you don't compile your production code with -O0, so let's try -O3. Here's f:

    f:
    .LFB0:
	movl	$42, %eax
	ret

And here's g:

    g:
    .LFB1:
	movl	$42, %eax
	ret

Notice how we're not calling f anymore; we just get f's body right in our function. So there's no speed cost to writing a function that only calls another function. This is the simplest possible case for an optimizer to optimize, and of course it does.

If you want to be explicit, you can always inline g. Then, at g's call site, we see:

    main:
    .LFB2:
	pushq	%rbp
	movq	%rsp, %rbp
	call	f
	movl	$0, %eax
	popq	%rbp
	ret

Which is almost the same thing as what -O3 produced, except g's body is copied into its caller instead of f's body being copied into g. (And this is at -O0.)

Another case where the thought process goes awry is the guess that there is a preprocessor bug. It's easy to verify, run gcc with -E and see what you get. Here's our test case:

    #define foo f

    int f(void){
       return 42;
    }

    int g(void){
      return foo();
    }

Preprocess:

   $ gcc -E buggy.c

    int f(void){
      return 42;
    }

    int g(void){
      return f();
    }

And look. No bug!

So let's move on to the final problem, the weird comment. We'll again see what our preprocessor output looks like on a minimal test case:

    #define FILE int

    #ifdef _WIN32
    #  define tmpfile w32_tmpfile
    #endif

    FILE *tmpfile(void) { return (FILE *) 0 }
    FILE *w32_tmpfile ( void ) { return (FILE *) 0 }

    void test(void){
      FILE *g;

      if (code == 200) {     // Downloading whole file
        /* Write new file (plus allow reading once we finish) */
        // FIXME Win32 native version fails here because Microsoft's version of tmpfile() creates the file in C:\
        g = fname ? fopen(fname, "w+") : tmpfile();
      }
    }

We end up with:

    int *tmpfile(void) { return (int *) 0 }
    int *w32_tmpfile ( void ) { return (int *) 0 }

    void test(void){
      int *g;

      if (code == 200) {



      }
    }

Which looks like our code has suspiciously gone missing. Let's try removing the comments:

    int *tmpfile(void) { return (int *) 0 }
    int *w32_tmpfile ( void ) { return (int *) 0 }

    void test(void){
      int *g;

      if (code == 200) {
        g = fname ? fopen(fname, "w+") : tmpfile();
      }
    }

And hey, our code is back. Must be something to do with the comments.

Anyway, the C toolchain is very simple, so let the computer do the compilation for you when you suspect the compiler isn't doing what you want. Read and understand its output; don't rely on a tool that does something that you couldn't do yourself.

Something I mentioned in that code completion article the other day was "don't rely on your editor to be your brain". This got a lot of downvotes, but this is exactly the problem the author has here. His editor didn't interpret the source code in the same way that the compiler did, and that's what matters.

Learn the language you are using so that you can spot bugs! Don't think something isn't a comment just because your editor doesn't show it in italic orange! Suspect an editor bug, not a compiler bug.

And finally, if you use correct grammar in your comments, this wouldn't have happened:

    /* We use our own tmpfile implementation here, since Windows' writes directly to C:/. */

Martin Fowler in "Refactoring" supported one of my prejudices, which is that "comments are bugs". So we could have nipped this bug in the beginning by not commenting here.

Then there is the question of why are we even thinking about the optimization issues about calling tmpfile? Are we doing billions of these calls? Or more on the order of tens of them. But in either case, wouldn't it be evident prior to any optimization or measuring effort that the time taken up by the OS to actually create the file would swamp any gain done by this optimization?

One might then argue that the bugs are 1) using comments 2) attempting an optimization that simple pre-calculation shows no real benefit.

emacs unfortunately fails on the trigraph-sequence "??/" and ofcourse the line immediately following doesn't get executed. having AST based syntax hi-lighting would of course fix the issue. most editors, afaik, used hacked together regexe's which break-down under various conditions.

Similar bug:

  int i = 10;
  int *p = &i;

  int j = 100/i; // j==10 right? right

  int k = 100/*p; // k==10 right?

That last line doesn't compile.

In addition to everything else, please don't use a negative condition in an if/else statement.

    if(NULL != fname) {
        g = fopen(fname, "w+");
    } else {
        g = tmpfile();
    }

Better:

    if(NULL == fname) {
        g = tmpfile();
    } else {
        g = fopen(fname, "w+");
    }

I loved the punchline, & probably wouldn't have spotted that one myself.

As others have noted, the moral is to not use C++ '//' style comments in C code or at least hope that you are using a clever editor ;)

Maybe this is why my C tutor used to say over and over again "never use C++ style comments even though Visual studio let's you do it".

The most stupid C bug ever? Pointers.

Or rather, value of, rather than dereference by default. Ada got this right, dereference by default, no pointer arithmetic.