7 ms·
The most stupid C bug ever
- burgerbrain 15y agoI'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).
- hartror 15y agoThat was my reaction. Syntax highlighting makes syntax errors a breeze to notice.
- Jach 15y agoI thought this as well, then added a point to "Backslash directory separators" on my list of reasons to go back in time to kill Bill Gates...
- gmac 15y agoExactly. This seems, first and foremost, a syntax highlighting bug.
- Wilfred 15y agoFor what it's worth: that's stock Emacs behaviour, at least on the current version (23.3). Seems like a highlighting bug.
- jrockway 15y ago"#if 0" is not how you declare comments in C. If you want your dead code to show up as a comment, try M-x comment-region. It adds /* */ around each line, being careful to ensure the ability to reverse the operation. I know vim doesn't have good commenting functionality in the core, so this is probably a common vim user idiom. But it's still not correct.
- dools 15y agoMaybe 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".
- jmj42 15y agoMy thoughts, exactly. Perhaps I'm a little overly pedantic, but it annoys me to extremes when I find c++ style comments littering up c
- burgerbrain 15y agoWhat I do is I turn on my compilers warnings for C++ comments in C code, then use C++ comments with all my TODO comments. That way I know if there are any outstanding TODOs when I go to build.
- mahmud 15y agoIt's not a warning in GCC. The -ansi flag, and older standards, err on C++ comments, but they're still valid input for C99 mode(s).
- burgerbrain 15y agoWith -pedantic you can get: warning: C++ style comments are not allowed in ISO C90 [enabled by default] Might be worth noting at this point since I'll already halfway though the GCC manpage again, that -Wcomments seems to be exactly for the sort of situation the author of the article found himself in.
- acqq 15y agoIt depends on what you write. If your goal isn't portability, using // has nothing to do with author's unawareness of the semantics of the line continuation character in C (that is, escaped line end character). It's a C feature since forever, and not knowing about it would bite him sooner or later even without the use of C++ comments.
- 15y ago
- angusgr 15y agoI 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.
- nandemo 15y agoIndeed. What's a wrapper function's overhead compared to opening a file, which subsequently will be written to? Frankly, I wish this sort of blog post wouldn't be upvoted here. It's one thing when someone asks a basic question on StackOverflow. Even experienced programmers can learn with the answers. But when someone writes an "authoritative" blog post that misses the basics, it's more noise than signal.
- snprbob86 15y ago"What to Submit On-Topic: Anything that good hackers would find interesting." [1] I agree about this being an insane premature optimization, but the C:\ being a bug regarding line continuation within comments? That's fucking fascinating. [1] http://ycombinator.com/newsguidelines.html http://ycombinator.com/newsguidelines.html
- TorKlingberg 15y agoTo be fair, the bug that the post is about has nothing to do with his optimization. That is just context.
- joshu 15y agoWell, jrockway's deconstruction was pretty nice to read even if the original was boring.
- nandemo 15y agoPoint taken. I learned something with the comments if not with the original article.
- splicer 15y agoCompiling 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.
- Jach 15y agoAlso -ansi and -pedantic if you're aiming at the C89 standard for some reason, it's great to look in the gcc manpage and see all the little flags that are around. I don't often like -Werror since it can interrupt my flow. If I'm testing runtime correctness, and something like "suggest parentheses around assignment used as truth value" pops up when that was intentional, I don't want to make the context switch to go back and fix it and recompile before context switching again back to test whatever runtime correctness I was aiming for. I'll fix it later. Philosophically, -Werror assumes I'm the type of person who ignores warnings completely for the duration of a project, and will stop you from ignoring them in the first place even if there may be reasons to; I don't think most people are that way.
- kmm 15y agoI wish they were standard GCC behaviour. Sometimes when I rapidly make a toy program, I forget something important, like including stdio.h and without warnings, I won't know until the program crashed.
- 5hoom 15y agoI 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 ;)
- StavrosK 15y agoIt would have produced the same bug in C++ code as well.
- 5hoom 15y agoI guess it would, it's just not something I've run into. The only time I've ever seen backslash's used in C or C++ as a line-break is in huge multi line macros with lots of variables that has been broken up for readability. It never occurred to me it could be used to do multi line comments but there you are.
- zvrba 15y ago"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).
- signa11 15y agoemacs 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.
- spitfire 15y agoThe 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.
- wlievens 15y agoReally? Can you do everything C can do on every platform it's used on, after you've hidden away pointers? Or do you simply want to change the syntax?
- keeperofdakeys 15y agoIt isn't a bug, it is a feature. C wasn't designed to abstract away the reality of pointers, which are the basic building blocks of assembly. If you don't like pointers, then don't use C. All programming languages have their time and place (even something like whitespace). The advantage of not abstracting pointers is that it makes low-level programming much easier. Instead of having to write your operating system kernel in assembly, you can write it in a portable language that is much easier to read!
- exDM69 15y agoEvery language which does not support pointers and pointer arithmetic by default has a more complicated way of doing it. Look at Python, for example. When you want to deal with arrays for interfacing with C code or image processing or just for plain performance gains, you have to deal with Numpy arrays which are even more difficult than pointers (esp. when it comes to C code wrappers). Haskell gets this right. You never write regular Haskell code with pointers but when you need to do some low level stuff, there is a native pointer type and I/O operations peek and poke. The pass by value semantics of C are well suited for the low level language that C is.
- chalst 15y agoThe 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.
- derleth 15y agoYou know how the usual wisdom is that you can never remove anything from a public interface? I think compiler writers should test that by removing trigraphs. Silently at first, to avoid panic, then announce that a mistake has been fixed where some groups of three characters were lexed improperly into a completely unrelated single character. Never mention that the mistake was in the standard. I seriously doubt anyone would care if you did it that way. (Modern versions of gcc (4.5.2 at least) ignore trigraphs by default but issue warnings saying they ignored one and giving the command line option to honor them. But gcc has long honored the dumber parts of the standard more in the breach than the observance.)
- sterwill 15y agoI've seen C code on EBCDIC machines that uses trigraphs because some code pages lack #, {, }, etc. Someone probably still needs them.
- kragen 15y agoGCC has digraphs that are much more readable than ANSI trigraphs.
- esrauch 15y agoAre EBCDIC machines such a common development platform that trigraphs should be default on instead of default off with the option to enable? Though I suspect trigraphs being enabled is a very rare source of bugs to begin with, it seems likely that it is still more common than the scenario of active development on platforms that don't support the normal characters (eg the actual development is done on the platform rather than cross-compiled).
- qntm 15y agoIn 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+"); }
- chalst 15y agoI don't agree here. != is a primitive, not a negative condition in the usual sense. Putting things this way around is natural, where the usual action appears first and the exceptional action appears later.
- arethuza 15y agoI'm not particularly disagreeing with the point you are making - but would it really be asking to much for people to provide explanations of why one form is preferred over another?
- qntm 15y agoCertainly. The else block in this case is executed when "NULL != fname" is false. That's an unnecessary double negative and takes a nonzero amount of mental effort to unscramble. The example I gave reverses this. This would also be acceptable: if(fname) { g = fopen(fname, "w+"); } else { g = tmpfile(); } (I also don't regard either statement as an "exceptional condition", they both seem equally likely to me.)
- arethuza 15y agoThanks - I've started getting a bit grumpy about coding rules that come without any explanation of why you should do something (simply saying something is "best practise" often being the worst "explanation" of all).
- VBprogrammer 15y agoFor what it's worth I couldn't disagree more. I use the guard pattern very often (as described here http://video.ias.edu/webfm_send/1207 http://video.ias.edu/webfm_send/1207 [Video]). Rather than writing: if (argc == 3) { if (fname) { /* Do stuff */ } else { printf ("File name cannot be null"); } } else { printf ("Wrong number of arguments"); } I would write: if (argc != 3) { printf ("Wrong number of arguments"); } else if (!fname) { printf ("File name cannot be null"); } else { /* Do stuff */ }
- gfodor 15y agoI too am tired of paying the price for C's notoriously expensive function calls.
- jrockway 15y agoEspecially around the otherwise speedy task of disk IO. On Windows.
- exDM69 15y ago"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.
- msftguy 15y agoEven without inlining, it will be subject to tail call elimination with any sane compiler: my_tmpfile will just branch to _imp_tmpfile instead or returning.
- jrockway 15y agoThis 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:/. */
- deleted 15y ago[deleted]
- JoeAltmaier 15y agoSimilar 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.
- reemrevnivek 15y agoUse spaces between arithmetic operators. You've already got them around the = sign. It looks much better, and prevents this problem: int i = 10; int *p = &i; int j = 100 / i; // j==10 right? right int k = 100 / *p; // k==10 right? right Also, any decent syntax highlighter would have caught the latter problem. The problem in the blog post was much more subtle, because most (all?) syntax highlighters won't catch it.
- wglb 15y agoMartin 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.