7 ms·

That source code looks HORRIBLE. Is it just because of the language or because the person who wrote had no other choice? http://git.savannah.gnu.org/cgit/emacs

That source code looks HORRIBLE. Is it just because of the language or because the person who wrote had no other choice? http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4887597a1c52b8277eabeb4d9484c75b1bc9429d http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=488759... It must have been done for a reason right, emacs is incredibly old in tech years and solid as a rock.

sputnik27 11y ago
Oh, god, so awful. All those ifdefs.. Funny: Tried to find the comment commenting it out. I found it. #IF 0. Wow.
- jff 11y ago
  That's a pretty standard way to comment out a huge chunk of code, because /* */ will get screwed up if there's another comment using the same convention inside, and putting // in front of every single line looks bad/makes copying code around painful. #if 0 is also a convenient way to flip bits of test/debug code on and off.
  - provemewrong 11y ago
    That's actually neat! Haven't touched C/C++ beyond university and didn't know about that.
- kbart 11y ago
  #if 0 <...> #else <...> #endif Is a pretty standard way to switch between new/old code while refactoring to check if behavior/output matches. Just change 0 to 1 to switch between "versions".
noobermin 11y ago
FWIW, I've seen worse code in closed source, computational codes. Still, I always thought GNU were in a better category than the rest of us.
- eropple 11y ago
  A short browse through gcc will strongly disabuse you of that notion. (Though some of that code is rotten for weird political reasons.)
  - LukeShu 11y ago
    It is my understanding that GCC was actually pretty clean until the EGCS fork.
    - duaneb 11y ago
      Yes. You can trace much of the shitty decision making to gcc 2.95.
    - zeckalpha 11y ago
      It is my understanding that EGCS was forked to clean GCC.
      - LukeShu 11y ago
        EGCS was all about generating faster code, adding features, et c.
pjc50 11y ago
String manipulation in C: not even once. It always ends up looking like this. This contains some niceties like `p[-1]` as well, which is perfectly valid but upsets people trying to check whether it's guaranteed to stay in the buffer. Quite a lot of the #ifdef damage is to deal with VMS.
ef4 11y ago
Pretty much all portable C from that era looks like that.
jheriko 11y ago
Actually this code /is/ horrible in many ways (single letter variable names instead of something descriptive?!?), as is 'the old style' in general - which is why those practices are uncommon or considered 'bad' today it can still be solid as a rock and be needlessly difficult to read, written in a poor style. they aren't mutually exclusive things, despite what many may preach.
mratzloff 11y ago
You may not be familiar with reading and writing C. String parsing in ANSI C in the 1980s looked like this. And it is completely readable and obvious to those who are well-versed in the language and practices of the time. For example, those naming practices were pretty essential when working with 80-character monitors. And all those ifdefs were required for dealing with DEC minicomputers and (later) microcomputers. A minicomputer, if you didn't know, was something that in terms of power and cost existed between a mainframe and microcomputer (i.e., a PC). Breaking out the code into different functions by platform (segmented by ifdef statements) would create unnecessary code bloat unless the implementations differed significantly. And believe it or not, this was much more readable (with the technology of the time) than calling separate functions within a master function, requiring referring to each function separately. Of course, ifdefs for different platforms are still required, and are often used in a similar way today. I guess what I'm trying to say here is that there were very practical and pragmatic reasons code used to look like this, and that things much more obscure and (to modern eyes) hard to read were written by very smart people who knew exactly what they were doing.
- pcwalton 11y ago
  To be fair, though, those smart people routinely wrote exploitable buffer overflows with code just like this (and "smart people" here is not sarcasm--they were smart!) C string handling may have been defensible 30 years ago (though I'm not sure about that), but not today.
  - ArkyBeagle 11y ago
    It wasn't defensible 30 years ago, either. I say that - may be I led a charmed life. Sigh.
  - TheCondor 11y ago
    You say that as if we don't have people writing buffer overflows and other exploits in C today.
  - mratzloff 11y ago
    Yep, you're absolutely right--but it was a time when the entire world wasn't networked to the degree it is today, and there was little or no motivation for someone thousands of miles away to gain access to your system. Security was simply less of a concern for most programmers, so much so that it often didn't even occur to them. Until the mid-1980s, even US federal law and its enforcement reflected that. Saying it is indefensible to manipulate strings in C is coming pretty close to saying it is indefensible to use C (and after they went to all that work to put together C11!). For most programmers, perhaps. But it still has its uses, and as of C11 safer standard string manipulation functions, as well. A competent, security-minded C programmer can write perfectly safe string handling code in modern C. Still, it wouldn't be my first choice. A Jedi may use a lightsaber effectively, but a novice will probably just end up cutting off his leg.
viraptor 11y ago
Old gnu software is full of code like that. If you want some nightmares, have a look at the source of tar, ar, or other tools. If you have to comment where the control goes back to your while(1) in a 200-line function (https://github.com/wertarbyte/tar/blob/ignore-missing/src/list.c#L492 https://github.com/wertarbyte/tar/blob/ignore-missing/src/li...), that's 100% a code smell.
jhallenworld 11y ago
Eh, the code is not that bad but it's weird that they don't scan from the right instead of the left to find 'xxx//yyy//zzz' (which means ignore 'xxx/' because you began a new path with '/yyy', but ignore '/yyy' also because you began a new path with '/zzz'). Here is the same code in JOE (which has no VMS support and which treats foo/../bar and bar as different files - the original emacs code was written before symlinks!). JOE has a variable length string library, so you see 'vsncpy' instead of the alloca / strcpy / strcat / make_string. http://sourceforge.net/p/joe-editor/mercurial/ci/default/tree/joe/b.c#l2534 http://sourceforge.net/p/joe-editor/mercurial/ci/default/tre... Edit: Holy Cow! The new version in emacs is much more involved: https://github.com/emacs-mirror/emacs/blob/master/src/fileio.c#L757 https://github.com/emacs-mirror/emacs/blob/master/src/fileio... "For technical reasons, this function can return correct but non-intuitive results for the root directory; for instance, (expand-file-name ".." "/") returns "/.."... " :-)
- JoshTriplett 11y ago
  > Eh, the code is not that bad but it's weird that they don't scan from the right instead of the left to find 'xxx//yyy//zzz' (which means ignore 'xxx/' because you began a new path with '/yyy', but ignore '/yyy' also because you began a new path with '/zzz'). expand-file-name doesn't do that; (expand-file-name "/foo//bar") produces "/foo/bar", just like any other pathname canonicalization. I can't help but wonder why emacs doesn't write that function in lisp, rather than C.
  - jhallenworld 11y ago
    You are right, there must be some other function which does that. Maybe the code predates the lisp interpreter.

sputnik27 11y ago

Oh, god, so awful. All those ifdefs.. Funny: Tried to find the comment commenting it out. I found it. #IF 0. Wow.

jff 11y ago
That's a pretty standard way to comment out a huge chunk of code, because /* */ will get screwed up if there's another comment using the same convention inside, and putting // in front of every single line looks bad/makes copying code around painful. #if 0 is also a convenient way to flip bits of test/debug code on and off.
- provemewrong 11y ago
  That's actually neat! Haven't touched C/C++ beyond university and didn't know about that.
kbart 11y ago
#if 0 <...> #else <...> #endif Is a pretty standard way to switch between new/old code while refactoring to check if behavior/output matches. Just change 0 to 1 to switch between "versions".

noobermin 11y ago

FWIW, I've seen worse code in closed source, computational codes. Still, I always thought GNU were in a better category than the rest of us.

eropple 11y ago
A short browse through gcc will strongly disabuse you of that notion. (Though some of that code is rotten for weird political reasons.)
- LukeShu 11y ago
  It is my understanding that GCC was actually pretty clean until the EGCS fork.
  - duaneb 11y ago
    Yes. You can trace much of the shitty decision making to gcc 2.95.
  - zeckalpha 11y ago
    It is my understanding that EGCS was forked to clean GCC.
    - LukeShu 11y ago
      EGCS was all about generating faster code, adding features, et c.

pjc50 11y ago

String manipulation in C: not even once. It always ends up looking like this. This contains some niceties like `p[-1]` as well, which is perfectly valid but upsets people trying to check whether it's guaranteed to stay in the buffer. Quite a lot of the #ifdef damage is to deal with VMS.

ef4 11y ago

Pretty much all portable C from that era looks like that.

jheriko 11y ago

Actually this code /is/ horrible in many ways (single letter variable names instead of something descriptive?!?), as is 'the old style' in general - which is why those practices are uncommon or considered 'bad' today it can still be solid as a rock and be needlessly difficult to read, written in a poor style. they aren't mutually exclusive things, despite what many may preach.

mratzloff 11y ago

You may not be familiar with reading and writing C. String parsing in ANSI C in the 1980s looked like this. And it is completely readable and obvious to those who are well-versed in the language and practices of the time. For example, those naming practices were pretty essential when working with 80-character monitors. And all those ifdefs were required for dealing with DEC minicomputers and (later) microcomputers. A minicomputer, if you didn't know, was something that in terms of power and cost existed between a mainframe and microcomputer (i.e., a PC). Breaking out the code into different functions by platform (segmented by ifdef statements) would create unnecessary code bloat unless the implementations differed significantly. And believe it or not, this was much more readable (with the technology of the time) than calling separate functions within a master function, requiring referring to each function separately. Of course, ifdefs for different platforms are still required, and are often used in a similar way today. I guess what I'm trying to say here is that there were very practical and pragmatic reasons code used to look like this, and that things much more obscure and (to modern eyes) hard to read were written by very smart people who knew exactly what they were doing.

pcwalton 11y ago
To be fair, though, those smart people routinely wrote exploitable buffer overflows with code just like this (and "smart people" here is not sarcasm--they were smart!) C string handling may have been defensible 30 years ago (though I'm not sure about that), but not today.
- ArkyBeagle 11y ago
  It wasn't defensible 30 years ago, either. I say that - may be I led a charmed life. Sigh.
- TheCondor 11y ago
  You say that as if we don't have people writing buffer overflows and other exploits in C today.
- mratzloff 11y ago
  Yep, you're absolutely right--but it was a time when the entire world wasn't networked to the degree it is today, and there was little or no motivation for someone thousands of miles away to gain access to your system. Security was simply less of a concern for most programmers, so much so that it often didn't even occur to them. Until the mid-1980s, even US federal law and its enforcement reflected that. Saying it is indefensible to manipulate strings in C is coming pretty close to saying it is indefensible to use C (and after they went to all that work to put together C11!). For most programmers, perhaps. But it still has its uses, and as of C11 safer standard string manipulation functions, as well. A competent, security-minded C programmer can write perfectly safe string handling code in modern C. Still, it wouldn't be my first choice. A Jedi may use a lightsaber effectively, but a novice will probably just end up cutting off his leg.

viraptor 11y ago

Old gnu software is full of code like that. If you want some nightmares, have a look at the source of tar, ar, or other tools. If you have to comment where the control goes back to your while(1) in a 200-line function (https://github.com/wertarbyte/tar/blob/ignore-missing/src/list.c#L492 https://github.com/wertarbyte/tar/blob/ignore-missing/src/li...), that's 100% a code smell.

jhallenworld 11y ago

Eh, the code is not that bad but it's weird that they don't scan from the right instead of the left to find 'xxx//yyy//zzz' (which means ignore 'xxx/' because you began a new path with '/yyy', but ignore '/yyy' also because you began a new path with '/zzz'). Here is the same code in JOE (which has no VMS support and which treats foo/../bar and bar as different files - the original emacs code was written before symlinks!). JOE has a variable length string library, so you see 'vsncpy' instead of the alloca / strcpy / strcat / make_string. http://sourceforge.net/p/joe-editor/mercurial/ci/default/tree/joe/b.c#l2534 http://sourceforge.net/p/joe-editor/mercurial/ci/default/tre... Edit: Holy Cow! The new version in emacs is much more involved: https://github.com/emacs-mirror/emacs/blob/master/src/fileio.c#L757 https://github.com/emacs-mirror/emacs/blob/master/src/fileio... "For technical reasons, this function can return correct but non-intuitive results for the root directory; for instance, (expand-file-name ".." "/") returns "/.."... " :-)

JoshTriplett 11y ago
> Eh, the code is not that bad but it's weird that they don't scan from the right instead of the left to find 'xxx//yyy//zzz' (which means ignore 'xxx/' because you began a new path with '/yyy', but ignore '/yyy' also because you began a new path with '/zzz'). expand-file-name doesn't do that; (expand-file-name "/foo//bar") produces "/foo/bar", just like any other pathname canonicalization. I can't help but wonder why emacs doesn't write that function in lisp, rather than C.
- jhallenworld 11y ago
  You are right, there must be some other function which does that. Maybe the code predates the lisp interpreter.