16 ms·
Fixing a 37-year-old bug by merging a 22-year-old fix
- ceejayoz 12y agoIn a 24-year-old version control system. edit: Wow, not sure what's offensive here...
- idlewords 12y agoYou're probably getting downvoted because a lot of OpenBSD threads derail into fruitless discussions of their use of CVS.
- deleted 12y ago[deleted]
- tux3 12y agoPeople here downvote what they don't like
- ceejayoz 12y agoI'm well aware of that, the question is why don't they? I found it a neat aspect that an long-lived bug with a long-lived PR is in a long-lived VCS.
- DanBC 12y agoYou're single line comment might have appeared dismissive of the version control used by the OpenBSD people, especially because it's something that often comes up in threads about BSD. Threads involving OpenBSD can involve fierce downvoting so it's probably a good idea to be clear about what you're saying.
- res0nat0r 12y agoI think all of this could be easily avoided if HN stopped allowing people to try and silence valid comments that they happen to disagree with. The site is now more political than ever and this just seems to fuel the "silence what you don't agree with" type voting, which I don't think is a good thing.
- gknoy 12y agoTopic aside, the original comment was ambiguous. Is it a middlebrow dismissal ("Pfft, their VCS is two dozen years old ..."), or is it an expression of admiration ("Wow, it's pretty amazing that their version control system has stood the test of time, even when the rest of the world seems to be moving towards git!")? The pythonic tendency towards being explicit about what we mean to say serves us well even in prose. :-) I agree with the OP, it's a pretty neat engineering feat. I think the downvotes would likely have been avoided had that admiration been more clearly stated.
- hnha 12y agoor what they consider irrelevant or noise.
- jnbiche 12y agoThat's only true in the first hour or two or a comment's existence. If you wait until most active HNers have had a chance to weigh in, it's almost always only the content-free, Reddit-style (eg., "that's true!") or blatantly offensive comments that are downvoted below 0.
- brynet 12y agoI posted this in the reddit thread today, after being part of a larger discussion last week. Theo de Raadt co-created AnonCVS back in 1995/1996. He and Chuck Cranor from AT&T Labs Research published a paper in 1999 about it. OpenBSD was the first open source project to make access to their source repository public to non-developers, before AnonCVS you needed commit access to review history, annotations and even checkout. OpenBSD had a read-only CVS tree even before their website was created in 1996. There are no known prior cited examples of a project providing this level of transparency to the development process, most open source projects at the time only provided "snapshots" of development in the form of compressed source directory tarballs, excluding RCS files. SUP and FreeBSD's ctm(1) only let you merge "current" remote changes to a local non-versioned directory. http://www.openbsd.org/papers/anoncvs-paper.pdf http://www.openbsd.org/papers/anoncvs-paper.pdf http://www.openbsd.org/papers/anoncvs-slides.pdf http://www.openbsd.org/papers/anoncvs-slides.pdf And here's an additional historical tidbit relating to FreeBSD and AnonCVS: https://twitter.com/canadianbryan/status/517714611089719296 https://twitter.com/canadianbryan/status/517714611089719296
- Scuds 12y ago> for (firsttime = 1; ; firsttime = 0) { does this line predate the while loop? or boolean values in C?
- staticshock 12y agowhile loop right above the for loop: > while ((ch = getopt(argc, argv, "n:")) != -1) { bool was introduced in C99
- piran 12y agoC99 introduced the boolean stdlib, so yes it predates that. Also, as the other guy said, that code has while loops.
- ghshephard 12y agoWhat's the common way of doing this in C? Is it normally something like this?: firsttime=True while (True) { .. .. .. firsttime=False }
- mbreese 12y agoThat's probably the cleaner way to do it, but honestly, the hacked for-loop feels pretty clean too. And it ensures that your firsttime flag is properly set, regardless of how you break out of the loop (a 'continue' in the middle of your example would fail to set the flag properly).
- Someone 12y agoDepends on the person. There likely is a correlation on what century you learned programming, but certainly one whether you have experience with pascal-ish languages. People educated in the Wirth way would say "that's not a for loop; there is no way to tell how often it will iterate when it is first entered" and write it as a while loop. Old-school C programmers seem to think one iteration construct is enough for everybody and like to write any iteration as a for loop, ideally with a few commas and no actual statements, as in the classic strcpy: char *strcpy(char *dst, CONST char *src) { char *save = dst; for (; (*dst = *src) != '\0'; ++src, ++dst); return save; } (from http://www.ethernut.de/api/strcpy_8c_source.html http://www.ethernut.de/api/strcpy_8c_source.html) Programmers with soe Wirth-style training do at least something like char * strcpy(char *s1, const char *s2) { char *s = s1; while ((*s++ = *s2++) != 0) ; return (s1); } (from http://www.opensource.apple.com/source/Libc/Libc-167/gen.subproj/ppc.subproj/strcpy.c http://www.opensource.apple.com/source/Libc/Libc-167/gen.sub...) [If you google, you can find way more variations on this. for example, http://fossies.org/dox/glibc-2.20/string_2strcpy_8c_source.html http://fossies.org/dox/glibc-2.20/string_2strcpy_8c_source.h... does a do {...} while(....) that I am not even sure is legal C; it uses offsets into the source to write into the destination] From your example I guess you are definitely in the Wirth (Pascalish) camp, beyond even that second state. Classic C programmers would either use 0 and 1 for booleans or #define macros FALSE and TRUE (modern C has true booleans, but that extra #include to get it increases compilation time, so why do it? I am in te Pascal-ish camp. I like to say C doesn't have a for loop because its 'for' is just syntactic sugar for a while loop.
- kazinator 12y agoThis link to the commit log is slightly better: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/head.c http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/he... You're one click away from the diff.
- 0x0 12y agoWhat's with the previous commit about a "DDOS" - it's just changing exit(0) into exit(status)?
- kazinator 12y agoThere must be more context to that. CVS doesn't have change sets; just from looking at this log, we don't know what else may be related to this. Maybe some important shell scripts hang in a loop because they expect head to report an unsuccessful termination status when a file doesn't exist. Maybe some situation exists where a privileged system script can be fooled into looping by an unprivileged user. Here is the mailing list discussion: https://www.mail-archive.com/misc@openbsd.org/msg132628.html https://www.mail-archive.com/misc@openbsd.org/msg132628.html There is no report of any actual DDoS. Craig Skinner was really just testing tools on nonexistent files! So the commit comment just follows from some general hypothesis that incorrect exit statuses from tools can be exploited in some way by attackers.
- 0x0 12y agoLooks like that's it, and the commit log was probably just sarcastic then. :)
- yelnatz 12y agoDiff: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/head.c.diff?r1=1.17&r2=1.18&f=h http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/he...
- kazinator 12y agoThis fix leaves a variable uninitialized, and depends on funny logic to later give it a value. The general structure should be FILE *fp = stdin; /* sane default */ then override with a new pointer from fopen if necessary. In fact, looking at this more closely, note how fp is scoped to the body of the for loop. That's where it should be defined: for (firsttime = 1; ; firsttime = 0) { FILE *fp = stdin; /* logic flips it to fp = fopen(...) as necessary */ } [Note: the C compiler Joy was working with probably didn't support block scope declarations except at the top of the function. This comment applies to merging the fix using the environment of today.] Note how the fclose at the bottom of the loop ends up closing stdin in the case that fp == stdin! It ensures that this only happens once, but you have to analyze it to see that this is true. I would write this as: if (fp != stdin) /* defensive! */ fclose(fp); Making it an infinite loop terminated by calling exit is unnecessary. There is already a continue in the loop, and a break in an inner loop; another break instead of the exit, and a "return status;" at the bottom wouldn't hurt. Better yet: /* Enter loop whenever there are args left to process, or even if there are no args if it is the first time through (to process stdin instead). */ for (firsttime = 1; firsttime || *argv; firsttime = 0) { FILE *fp = stdin; if (*argv) { /* have args; do the fopen to replace stdin */ /* set status = 1 and continue if cannot fopen */ } /* do head logic */ if (fp != stdin) fclose(fp); } return status; C itself was only a couple of years old when that code was written originally, and the systems were small. Whereas I've been using it since 1989. Not criticizing the original!
- michael_h 12y agoAs I understand it, you can fclose(stdin) without issue on almost all POSIX systems.
- kazinator 12y agoYes, I know this, and in a throwaway little program where you are sure you are the last one to ever touch stdin (because you're calling exit right there in the loop, for instance) it is okay.
- cordite 12y agoOne of my coworkers found a data integrity causing bug that has been around for 11 years. He and his team lead had no idea how such a bug missed 11 years of peer-QA through all edits on that module / activity. It was as simple as converting to string and back and hitting an edge case on internationalization with commas vs periods.
- burtonator 12y agoThose are the ones which scare me the most.. when you find them, they're potentially devastating, and you haven't hit them. Another I've seen is the potential landmine that can go of at ANY time and then one day, 5 years in the future, it hits you at 2AM and you can't figure out WTF is happening.
- pacaro 12y agoMy favourite decimal separater i18n bug was in a product where we were reading a time dilation factor from a config file. In the default shipping file (which basically nobody ever changed) it was specified as "1.0". We would then read this using (this is in .Net) Double.parse() which honors the users locale if none is specified, so for all users in Europe, and probably elsewhere, this was interpreted as 10 ('.' is a grouping construct and more or less ignored). So the software was running 10x slower than expected.
- MichaelGG 12y agoI shipped a program with the exact save bug! And I've worked on at least one website with the same bug. ASP.NET uses HTTP headers to determine locale and will parse/format on it. Which means if someone used the currency specifier for some reason, all of a sudden they're displaying the wrong prices! Or, if the website reads some string from the back end, like say a config value "DefaultCreditLimit", the user night be able to modify the interpretation of it. I've come to the conclusion that this automatic behavior that tries to guess at the format required is simply wrong. If a program wants user-specific formatting, it should opt-in.
- darklajid 12y ago. would be a weird here as well (and we had a share of issues with early online banking systems: Think "I want to pay 10 $currency, let's put 10.00 - and the bank uses the . as a delimiter for thousands, i.e. 1.000,00 / you meant 10,00). That said, my favorite localization issues stem from CH/Switzerland. Numbers are formatted as 1'000.00 there (someone correct me), which is especially odd: 1) That's a weird decimal separator. DE uses a , and as far as I know FR uses a , as well. I _assume_ IT uses a , - so why is this a . in CH? 2) The ' really kills a lot of naive parsers and causes quite a bit of frustration in other fields (think OCR - some overly fat 1'000 might turn into a 11000 with bad luck/crappy image quality or (marginally better) lead to 1000 where means 'I think here's a character, but no clue what that might be')
- ape4 12y agoI like that the -number option is "obsolete". I still use it.
- alexmchale 12y agoThat's pretty funny. I'd say that the majority of the time that I call head or tail, it's using that syntax.
- cerberusss 12y agoExactly, and I bet that it's not going away any time soon, "obsolete" or not.
- gojomo 12y agoIf OpenBSD has anyone young enough, would've been a fun twist to have someone who wasn't even born at the time of the fix make the commit.
- ahmett 12y agoHere is the diff: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/head.c.diff?r1=1.17&r2=1.18&f=h http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/head/he...
- userbinator 12y agoThe extremely odd for-loop structure feels to me like someone was trying very hard to avoid a goto, and in the process obfuscated the flow quite a bit more. The special case of head'ing stdin is similar to the classic "loop and a half problem" (which is really only a problem if you religiously abstain from goto use.) I'd consider this a more straightforward way to do it: FILE *f; int i = 1; ... if(argc < 2) { f = stdin; goto do_head; } ... while(i<argc) { f = fopen(argv[i], "r"); /* check for failure to open */ ... do_head: /* head'ing code goes here */ ... fclose(f); i++; } That being said, OpenBSD code is still far more concise and readable than GNU; compare coreutils' head.c here: http://code.metager.de/source/xref/gnu/coreutils/src/head.c http://code.metager.de/source/xref/gnu/coreutils/src/head.c GNU head has a bit more functionality, but I don't think the increase in complexity - nearly 10x more lines - is proportional to that.
- Flow 12y agoHere's the version used in OS X. It's a bit bigger but not GNUishly big. http://www.opensource.apple.com/source/text_cmds/text_cmds-87/head/head.c http://www.opensource.apple.com/source/text_cmds/text_cmds-8...
- justincormack 12y agoThat's almost certainly the FreeBSD one.
- userbinator 12y agoLooks like a small extension to the BSD one, that can also handle head'ing by byte count. (Although I think the use of malloc() and strcpy() to handle the -[0-9]+ case seems rather unnecessary...)
- aalbertson 12y agoLooking through all these comments, my only response is "effing nerds..." <3