Comment by fexl | Hacker News Reader

Comment by fexl | Hacker News Reader

Parent Full thread

I love C but I use it in a disciplined way. Here's the email I sent to the author:

James:

I liked your article on range checking.

I'm very keen on assertions, for example here:

https://github.com/chkoreff/Fexl/blob/fresh/base/src/buf.c#L...

It's still fast as greased lightning. I buffer up 2.6 MB here:

https://github.com/chkoreff/Fexl/blob/fresh/test/src/run.c#L...

It's too fast to notice. I can bump it up to 260 MB and try that:

  $ ./build && time ../bin/run
  : Buffering 260000000 bytes
    length = 260000000

  real	0m1.751s
  user	0m1.608s
  sys	0m0.140s

What the heck, let's try an even 2 GB:

  $ ./build && time ../bin/run
  Compile run
  Link run
  : Buffering 2000000000 bytes
    length = 2000000000

  real	0m13.350s
  user	0m12.049s
  sys	0m1.288s

Ah, but what if I remove my range check?

  $ ./build && time ../bin/run
  Compile buf
  Link run
  : Buffering 2000000000 bytes
    length = 2000000000

  real	0m13.103s
  user	0m11.781s
  sys	0m1.312s

Repeated testing of both ways shows no significant difference.

I think I'll leave in the range check.

Range checks also support one of my favorite programming maxims:

  No Silent Failure!

How will assert help if the failure happens in production code? Wouldn't it be better if the error was logged to a file.

Assertion failure messages are sent to stderr. Production code can capture that as it wishes.

The assert from assert.h is a macro that completely elides the check if built with NDEBUG. Assuming you're not using some other assert(), and are building your production build with NDEBUG (which is typically part of what's meant by "a production build", though you can certainly disagree with the practice), production code cannot capture it as no messages will be generated.

Right now, in my code, assert(0==1) produces this output to stderr:

  run: run.c:422: main: Assertion `0==1' failed.
  Aborted

That is what I want.

You are correct. If I build with -DNDEBUG=1, then the assert is completely ignored: no error message, and no abort. That is not what I want. Therefore I will not build with -DNDEBUG=1.

I will certainly not introduce "logging to a file" as a concept inside buf.c. There is no need for the pure data manipulation code in buf.c to know anything about files or stdio, or a specific name of a log file.

If the assert from assert.h does not do what I want, then I'll make a version that does. However, that is a moot point, since right now it does what I want.

I had no objection to the approach, just the lack of a note about assumptions it relied upon.

No it's a good point actually, and it pays to know your context. Frankly, looking at the 118 lines in assert.h, it does considerably more than I really need.

Instead of this:

  assert(buf->pos < buf->str->len);

I could just do this instead:

  if (buf->pos >= buf->str->len) die("bad pos");

If the die message is unique, I don't really need to include __FILE__, __LINE__, and the expression itself in it.

I could even do this, though it's probably overkill for something that should never happen anyway:

  if (buf->pos >= buf->str->len)
      die("bad pos %d %d", buf->pos, buf->str->len);

Absolutely, though I tend to throw __FILE__ and __LINE__ in everything, so I can step through the sources of messages by just pulling messages into my vim quickfix buffer. Again, totally depends on context, though.

For in-house code in a manufacturing company, we usually left all the debug symbol and assertions in our C code. For writing a product that shipped to others at a dev tools company, we usually turned off the debugging, turned on NDEBUG and full optimization. (although the product had checking for things built into its logic, and a lot of testing -- C being used as an assembler surrogate -- as we were writing developer tools, rather than "enterprise" whack-it-together-on-no-time-budget pile-age)

"Production build" varies by environment, TMTOWTDI.

Absolutely the case. I just think it's wrong to assume - without stating it - that NDEBUG will be off "in production".

Reply on news.ycombinator.com