Deconstructing "K&R C"

http://c.learncodethehardway.org/book/learn-c-the-hard-waych55.html

by gnufs • 15 years ago

118 129 15 years ago

c.learncodethehardway.org

This is just the first section of the chapter, right? It seems a little premature to be distributing or discussing it here.

calibraxis• 15 years ago

Good point. But that's the most inspiring approach I can remember ever seeing in such a book. If only all of my technical books were written with such a mindset.

zedshaw20• 15 years ago

Thanks, I do like breaking things.

jemfinch2• 15 years ago

Isn't "assert(line != NULL && longest != NULL && "memory error");" a bug in Zed's example code?

tba2• 15 years ago

No? It ensures that malloc didn't return a NULL pointer and the '&& "memory error"' is a common pattern to add a comment describing why an assert() statement failed.

kisielk2• 15 years ago

One problem with it is that on most compilers if you compile with optimizations it will remove the assertion. Now the code is no longer guarded against malloc failures and will just segfault. If the intent is to teach people how to handle malloc failures gracefully, it's not that great of an example.

chc• 15 years ago

I think you're mentally substituting "&&" with "||", in which case the assert would never fail because the string "memory error" is never 0. But with &&, the assert will fail when it should and the "memory error" is just a superfluous comment, not a bug.

jemfinch2• 15 years ago

That's absolutely correct, thanks for pointing that out.

tba2• 15 years ago

I'm confused. What is the "defect" in K&R's "copy(char to[], char from[])" function?

The author notes that "the second this function is called...without a trailing '\0' character, then you'll hit difficult to debug errors", but no function with that signature could possibly work in this case.

The built-in "strcpy" function has the exact same limitation. Does the author have a problem with it as well? Null-termination is a fundamental concept of C strings; there's no reason to shield C students from it.

The other example of "bugs and bad style" in this "destruction" of K&R C is a minor complaint about not using an optional set of braces.

I hope the remainder of the [incomplete] chapter demonstrates some actual bugs in the book's code, because it currently doesn't live up to the first paragraph's bluster.

angersock4• 15 years ago

It's a nice strawman, right? Especially when he points out that the original code, in context, is perfectly fine. His later complaint about the assignment-in-if statement is certainly something shared by modern C programmers (see compiler warnings about same), but it perfectly fits the original style and accomplishes its task.

His criticisms seem to be rooted so far in stylistic issues and in taking the code out of context (design context, usage guarantees, etc.). Then again, how are you to nerdbait while still being fair to original sources?

zedshaw20• 15 years ago

My criticisms are only partially stylistic, but also that people copy this code to other situations and it breaks. So, I'm showing them why it only works in that one specific context and how to break the code.

mechanical_fish2• 15 years ago

The built-in "strcpy" function has the exact same limitation. Does the author have a problem with it as well?

Yes. From the linked chapter:

we avoided classic style C strings in this book

From an earlier chapter on strings:

The source of almost all bugs in C come from forgetting to have enough space, or forgetting to put a '\0' at the end of a string. In fact it's so common and hard to get right that the majority of good C code just doesn't use C style strings. In later exercises we'll actually learn how to avoid C strings completely.

This is the author's opinion, of course – it's from a book, that should go without saying – but it's not as if the idea of avoiding C strings in general, and "strcpy" in particular, is an oddball or unique point of view. See e.g.:

http://stackoverflow.com/questions/610238/c-strcpy-evil

peapicker2• 15 years ago

"the majority of good C code just doesn't use C style strings..."

Nice. In the 23 years I have worked on C language products, I've never worked on "good C code" by this definition.

The cool thing about this guys book, I guess, is that by avoiding all the things about the language he doesn't like, any reader will be wholly unprepared for C in the Real World after this book.

shabble• 15 years ago

I didn't see it yet from a quick skim of the available chapters, but I'm going to guess that his preferred alternative is 'bstr - The Better String Library' - http://bstring.sourceforge.net/

I've used it the past, and I try to include it in any project that isn't immediately trivial, although the to/from integration with just about all library code ever is a bit of a hassle.

geophile• 15 years ago

Huh? My C programming is in the distant past, so I might be forgetting. But strcpy does assume a terminal zero, doesn't it? E.g., http://linux.die.net/man/3/strcpy. It sounds like you are talking about strncpy or memcpy.

blix2• 15 years ago

I'm confused by your comment. strcpy assumes that the string to be copied ends with a NUL. The case described in the link violated that assumption and caused a segfault.

zedshaw20• 15 years ago

Yes, strcpy is almost universally understood as a security problem. In fact, here's a nice regex to apply to some C code to find buffer overflows:

LC_ALL=C egrep '[^_.>a-zA-Z0-9](str(n?cpy|n?cat|xfrm|n?dup|str|pbrk|tok|_)|stpn?cpy|r?index[^.]|a?sn?printf|byte_)' src/*.c

Taken from the really well researched and secure andhttpd:

http://www.and.org/and-httpd/#secure

Run that regex on some C code, then go look at how the inputs to those functions are used, and then you can probably create some of your own buffer overflows. It's like magic.

X-Istence• 15 years ago

The other time that there will be an issue is when your to is shorter than your from...

zedshaw20• 15 years ago

> but no function with that signature could possibly work in this case.

This is the source of the bugs in C. People write functions that only work given all calls to them are never changed, which is absurd. Good modern C code involves trying to protect against bad usage and adding defensive checks.

So yes, the built-in strcpy is crap which is why most competent C doesn't use it except in a few rare cases where it's required.

And this does demonstrate actual bugs in the code. I wrote a test case that causes it, which incidentally is a common bug in C code called a buffer overflow. It's because of code examples like this that get copied to other situations that we have these defects.

tptacek17• 15 years ago

High level string libraries are a win.

But you may be overstating your case a bit.

From my codebase/third-party directory on my laptop (a bit random, I admit), from those projects I'd consider "competent C" (ie, not OpenSSL or MRI ruby):

* dovecot uses ASCIIZ strings and libc string functions

* redis uses ASCIIZ strings and libc string functions

* nginx uses a high-level buffered string library

* lcamtuf's skipfish scanner uses ASCIIZ strings and libc string functions

* libevent uses ASCIIZ strings and libc string functions

* qmail uses djb's string library

* memcached uses ASCIIZ strings and libc string functions

It's probably good to be comfortable with both approaches.

I don't know that you actually made this claim, but you seem to have given people here the impression that you believe functions that work with ASCIIZ strings should be bulletproofed to handle non-ASCIIZ inputs. I couldn't agree with that argument, especially as an argument about K&R's code being rusty.

People here are jumpy though (they're commenting, like me, mostly because they're bored).

Looking forward to more examples from the book.

gonzo3• 15 years ago

this from the man who won't describe xargs to newbies because he doesn't understand it.

blargh

blix2• 15 years ago

I don't see what the big deal is here. He throws inputs at a function not meant to handle them and gets a segfault. Isn't this more-or-less expected from all C functions?

On careful examination of his example code, you can see that there's no attempt to terminate the buffer (though he misidentifies it as an "off by one" error). Throwing arbitrary buffers into functions meant for null-terminate strings will cause errors everywhere, not just in K&R example code. In the next chapter, does he use strcpy in the same example and use that error to say that we should deprecate the entire standard library?

On a more nit-picky note, the 'triple-equality trick' the author derides is a common C idiom and one every C programmer should be familiar with. Perhaps it doesn't belong in the first chapter, but it definitely belongs in any C manual.

jpablo2• 15 years ago

Agreed, the example used seems very strange. Every function makes functions about it's parameters and in C null terminated string are to be assumed unless otherwise noted. The std library strcpy would just behave in an identical way.

Yeah, I'm all about checking your assumptions and defensive programming but k&r is hardly as fault because you throw garbage at a random function and it segfaults.

I find the idea of teaching modern C by pointing problems k&r awesome, but the examples given are not really good.

angersock4• 15 years ago

Yeah, I'm all about checking your assumptions and defensive programming but k&r is hardly as fault because you throw garbage at a random function and it segfaults.

Maybe he should write a series on "Learn Ada the Hard Way"?

zedshaw20• 15 years ago

You should go read about fuzzing. It's amazing what throwing "random garbage" at programs can break.

sliverstorm2• 15 years ago

I don't see what the big deal is here. He throws inputs at a function not meant to handle them and gets a segfault. Isn't this more-or-less expected from all C functions?

We are getting separated from that kind of behavior. Nowadays the language is generally expected to protect you from yourself.

marshray4• 15 years ago

No, C is specifically for situations where you don't want the language to "protect you from yourself". E.g. systems programming where little or no runtime performance will be sacrificed to enable imperfect code to be safe.

zedshaw20• 15 years ago

The inputs I'm throwing at it are a common problem mistake in C programming which demonstrates a buffer overflow. K&R's code assumes that it will only be used in each program they've written, but it usually get copied out into other software and that causes the bugs. As I said, the programs are correct, the functions are not.

And no, most of the clever convoluted code in K&R is antiquated and the only reason it's a common C idiom is because nobody questions whether it's good style. It's more of a habit than an idiom.

16s• 15 years ago

The greatness of the book is its terseness. Some of the code examples may be dated and buggy, but that does not diminish it at all.

bonzoesc5• 15 years ago

Dated, buggy, and un-stylish examples are exactly what diminishes it.

tptacek17• 15 years ago

And those dated, buggy, and unstylish examples would be?

mhartl• 15 years ago

It's worth noting that this isn't a standalone article; it's lesson 55 in "Learn C the Hard Way". In other words, this isn't meant to be a thorough critique. It's just one lesson of many.

askmeaboutloom2• 15 years ago

I don't see his point, really. Yes, C doesn't have a statically testable string type. And yes, the convention is that a C "string" is just an array of character data with a trailing NUL-Byte. He constructs an array without a trailing NUL-Byte - so that's not a string, but an array of characters.

The fact that copy() now happily runs through memory is the expected result of the bug in the calling code. No, it's not useful. Yes, there are problems with the whole approach of using a terminating value - but this doesn't seem to be his point (otherwise he would also have mentioned the linear runtime complexity of strlen() and the problems that arise when a string itself contains a NUL-byte, I suppose).

Now, what is his point? This is chapter 55 [!] in a book called "Learn C The Hard Way" and the author complains about well-know problems with the standard-lib string convention, optional braces, and the common C idiom of doing assignment and value-testing at the same time, _and_ calls all of this 'Deconstructing "K&R C"'?

Maybe a "What I personally don't like about C" would have been a better title. The K&R examples are flawless. The language and stdlib are not. That's well-known. What is new?

Turing_Machine• 15 years ago

"Yes, there are problems with the whole approach of using a terminating value" And, of course, there are problems with the other approaches too. Using a single length byte at the beginning limits you to 255 characters and makes pointer manipulation less straightforward (since the actual data starts one byte further on). Using more than one byte for the string length wastes space (a serious issue at the time C was developed, and even today in the microcontroller world). The author's assumption that he's smarter than Kernighan, Ritchie, and Thompson probably isn't the best approach.

By the way, the "old style of "leaving the cleanup to the OS" doesn't work in the modern world the way it did back in the day." works just fine on iOS and many other modern platforms.

tptacek17• 15 years ago

It's way worse than an extra byte, or the offset of 1 byte for pointers; it also means you need a whole copy of every substring with its own length delimiter, and can't tokenize in place.

C code gets into just as much trouble with length-delimited data structures as it does with ASCIIZ; ASCIIZ is a red herring. People have declared over and over again that it's the single worst decision in C and the cause of every buffer overflow. But if you look over the past 10 years or so, memcpy() has caused just as much chaos, and we're just as likely to find an overflow or integer mistake in binary protocols (where NUL-termination is nonsensical) as we are in ASCII protocols.

"Leaving the cleanup to the OS" works everywhere, on every modern system, and lots of programs would benefit from shedding a lot of useless bookkeeping and just treating the process address space as an arena to be torn down all at once. But I think the point the author was trying to make is, when you code that way, you make it impossible to hoist your code into another program as a module. Which is true; if it's likely you're writing a library as well as a program, you don't get to take the easy way out.

You can still write a 100 line arena allocator to pretend like you can, though. :)

zedshaw20• 15 years ago

> I don't see his point, really.

The point is three fold:

1. K&R has defects in it when the functions in it are used out of context because they didn't include defensive programming practices considered standard today.

2. People can learn a lot about writing good code by critiquing other code, even from masters, so I'm taking them through doing that.

3. There should be no sacred cows, and people hold K&R on a pedestal without questioning what's in it. This is really what causes #1, so I have them do #2.

That's it really.

michaelfeathers• 15 years ago

I'd rather see a book on writing C code the Zed way.

askmeaboutloom2• 15 years ago

Noble goals. And now that you said that I don't understand how I could have overlooked that in the first place.

Maybe it would be a good idea to put a bit more stress on the "no" in "no sacred cows", that's an important point beyond just K&R. Nothing should be sacred, including "Learning C The Hard Way" - and a "question everything and everyone" mindset is generally a good thing to have as a programmer [I think I read that in "The Pragmatic Programmer" ;-)]. But other than that I now think that chapter is fine. I do apologize for the somewhat passive-aggressive form of my question.

Thanks a lot!

adavies42• 15 years ago

> Braces Are Free, Use Them

I disagree. Braces have a cognitive load, particularly if they're given whole lines to themselves. Which is easier to read,

  while ((len = getline(line, MAXLINE)) > 0)
      if (len > max) {
          max = len;
          copy(longest, line);
      }
  if (max > 0) /* there was a line */
      printf("%s", longest);

  while ((len = getline(line, MAXLINE)) > 0)
  {
      if (len > max)
      {
          max = len;
          copy(longest, line);
      }
  }
  if (max > 0) /* there was a line */
  {
      printf("%s", longest);
  }

? Personally, I'd probably write it

  while ((len = getline(line, MAXLINE)) > 0)
      if (len > max) {
          max = len;
          copy(longest, line);
      }
  if (max > 0) printf("%s", longest);

. Consistent structural indenting makes it easy to see the boundaries of the control structures.

SeanLuke2• 15 years ago

I'm probably the last person in the world to prefer:

      while ((len = getline(line, MAXLINE)) > 0)
          if (len > max)
              {
              max = len;
              copy(longest, line);
              }
    
      if (max > 0) /* there was a line */
          printf("%s", longest);

I fear this may be literally true: this brace style is called Whitesmith's, and I was reporting bugs to the cc-mode indenter for emacs a while ago. BSD/Allman and K&R styles never made sense to me.

glitch• 15 years ago

SeanLuke, that is exactly how I'd write it too! ✼Whitesmith fistbump✼ — Related: I love the Wikipedia talk section on it: http://en.wikipedia.org/wiki/Talk:Indent_style#Whitesmiths_s...

Roboprog7• 15 years ago

Steve McConnell makes a pretty good case for Whitesmith's formatting in this book (at least the first edition) in the code formatting chapter:

http://books.google.com/books/about/Code_complete.html?id=Qn...

Of course, he presents most of the major styles, pretending to be "fair and balanced", but if you read between the lines of the "abstract block" argument, it's clear that the other styles which align keywords or grouping symbols at a different level other than the compound statements they delimit are brain damaged, or at least visually misleading.

zedshaw20• 15 years ago

Add a new statement to the while-loop. Now you've figured out why that style isn't so hot. You now have to remember to go add the new braces, and typically people are mentally putting them in so they forget. Same with the last if-statement. You'll go, add the new line, indent and go "cool done". Then scratch your head for hours wondering why it's not working.

NickPollard• 15 years ago

I'm currently experimenting with a hybrid of this and K&R style, that would look like this:

  while ((len = getline(line, MAXLINE)) > 0)
      if (len > max) {
          max = len;
          copy(longest, line);
          }
    
      if (max > 0) /* there was a line */
          printf("%s", longest);

So far I'm finding that it gives good readability, doesn't waste too many lines on braces, and doesn't run into many issues of accidentally leaving braces out.

I started it after having started writing lisp and python, and I'm getting used to just ignoring the braces and going by indentation, which this style emphasises.

ufo• 15 years ago

We are talking abou K&R here. Please use the less spacious TrueBraceStyle instead of the heretic Allman style. :)

Joke aside, the problem I see in your argument is that lack of braces is also a cognitive load. If braces are always there I can safely ignore them but if they can be ommited I need to worry if a certain piece if code is braced or not when I read and edit it.

The only exception I currently live with is single line if statements, as long as the statement is on the same line as the `if`

lurker14• 15 years ago

The middle version uses braces nicely to add whitespace to the function to help the reader flow through the code, instead of chasing matched-parens and wondering if each line is a continuation of the previous.

scott_s4• 15 years ago

Because it's the style I use, and because I don't need to bug-check the indented code to make sure it's braced correctly, I find this easiest to read:

  while ((len = getline(line, MAXLINE)) > 0) {
      if (len > max) {
          max = len;
          copy(longest, line);
      }
  }
  if (max > 0) { /* there was a line */
      printf("%s", longest);
  }

silon3• 15 years ago

By far.

cbs2• 15 years ago

Easiest to read? The middle. The way I would write it for my own use is closest to the top, but the easiest to read is the middle.

Always using braces, and giving them their own lines makes nesting as unambiguous as possible and the extra white space reduces density making foreign code easier to digest.

nessus42• 15 years ago

All this talk about where the right place to put braces is, makes me pine for Python.

zedshaw20• 15 years ago

> Braces have a cognitive load, particularly if they're given whole lines to themselves. Which is easier to read,

Checking braces has a higher load when they aren't there and when they aren't consistently used. But, without evidence both of our points are pointless. So, in my book I'm pushing the standards of always use braces except in a few little cases.

mindcrime• 15 years ago

I'll take the 2nd one, please. And twice on Sunday.

Teckla• 15 years ago

When it comes to assignment and comparison in constructs such as while loops, I think this variation adds readability:

while (len = getline(line, MAXLINE), len > 0) ...

I've never seen anyone else use it, though.

cbs2• 15 years ago

Guys, no need to jump all over this, clean code and defensive programming are solid practices to advocate. "That's just the way we do C" isn't really a good defense to someone trying to point out the warts in the way we do C, even if it did come from K&R.

Its easy to point to this for not sufficiently proving its argument, but its a work in progress, you're not the target audience, and finally, some of the shit in K&R seriously would not pass a code review here, and probably wouldn't where you work either.

MattyDub• 15 years ago

I think people are misunderstanding this; it's pedagogical. From the link:

'I want to use it as an exercise for you in finding hacks, attacks, defects, and bugs by going through "K&R C" to break all the code...When you are done doing this, you will have a finely honed eye for defect.'

He's trying to train his readers to think about places code can break.

tylerc230• 15 years ago

"Before the switch to heap memory, this program probably ran just fine because the stack allocated memory will probably have a '\0' character at the end on accident."

Stack memory is not preinitialized to '0' either so using stack memory would show this 'defect' as well (as others have pointed out this is a defect in the calling code, not in copy).

apu• 15 years ago

I think that depends on compiler, although it's been a while since I've done C, so I might be wrong.

zedshaw20• 15 years ago

It's fairly vague in the standard, but most of the time this is what happens so nobody hits the bug. Also, many OS are notorious for being loose with the stack and strict on the heap for various reasons.

Roboprog7• 15 years ago

Even if the stack was 0 filled at startup, it won't be after you call something else, and that other code returns. The stack will simply be loaded with the final values of auto vars from the called routines, as well as return addresses and other such stack frame tracking.

squiggly101• 15 years ago

Kind-of a long-winded way to point out the (very well known) fact that passing non-terminated strings to functions that expect terminated strings is fraught with danger, isn't it?

zedshaw20• 15 years ago

Nope, and what I'm actually doing is fuzzing the input to this function, which is a common breaking technique.

You see, programmers tend to go around only using functions exactly as they were intended. Hackers come along and then use them in unintended ways, which causes bugs. The way to prevent this is to think like a hacker and try to use your code in unintended ways and then try to prevent them.

gonzo3• 15 years ago

You say 'hackers' like it's a bad thing. As a reminder, this is "Hacker News".

halayli2• 15 years ago

The function copy expects a C style string but he pass a buffer to some data. If you pass it the proper argument you'll get a proper result, it's that simple.

I see the complaint is more about C style strings in general because it's easy to corrupt them by overwriting NULL at the end rather than a complaint about "K&R C" book. I don't get why he decided to put it under this title.

zedshaw20• 15 years ago

The problem is in the word "expects". This function expects certain things but doesn't assert that they're happening. Because of this you get bugs when you try to use that function in other code, which is what beginners do.

And yes, the solution is to require a size with every string buffer, which is what I do nearly all the time now. But, the best way to learn why that's solution is to try to hack something with this kind of defect.

halayli2• 15 years ago

I think the core issue here is how C style strings are implemented. Personally, I much prefer if C strings were <size><data>, but what I was going after is that it's not K&R specific, it's a C design "mistake".

Roboprog7• 15 years ago

To be honest, I was never that impressed with the K&R book as an introduction. Coming from a higher level language (Pascal) in college after several years back down to C was painful. K&R (borrowed from a classmate) presumed too much obviousness in what assembly language constructs were being abbreviated by a given piece of C code, and what the other pieces of C code around the example to be explained did.

Later in the semester I picked up a copy of the first edition of this book:

http://books.google.com/books/about/C_an_advanced_introducti...

This book did a much better job of explaining what the hell was going on, particularly in regards to managing memory / strings / arrays / (C) pointers and understanding their placement.

Sure, Pascal had pointers, when you asked for them :-)

Yeah, throw the K&R book at newbs you hate.

perfunctory3• 15 years ago

Not only the book "needs to be taken down from its pedestal" but it's time for the C language itself to go. It was fine language at the time (better than assembly I guess). No more. Not in the 21st century. The most common excuse for sticking with C is efficiency - I pity those who still believe so. Another excuse is extended set of libraries - http://go-lang.cat-v.org/pure-go-libs, when Go was publicly released?

That is, stop writing books about C and start creating new languages.

perfunctory3• 15 years ago

A lot of C lovers I see.

richardk• 15 years ago

Myself, I learned from K&R, and I thought it was great at the time, now I'm less sure of this.

I've heard that "A Book on C" is supposed to be a much better book for learning C

gonzo3• 15 years ago

I'm sure I'll be downvoted, but C is not Java.

From what I can tell, the "it's time for C to go" comes from a generation raised on Java.

C still has its place.

Deconstructing "K&R C"

Related Stories