Guide to Advanced Programming in C

http://pfacka.binaryparadise.com/articles/guide-to-advanced-programming-in-C.html

by pfacka • 12 years ago

168 74 12 years ago

pfacka.binaryparadise.com

sdegutis2• 12 years ago

I like the idea behind this article, but I'm a little skeptical of some of its advice.

For instance, I don't think it's a good idea to recommend using Boehm GC as a general solution to avoid memory management. It can't really tell the difference between pointers and pointer-sized integers, which means it usually leaks memory.

pfacka10• 12 years ago

I see, but could you please provide better alternative? I couldn't find anything more alive and representative than Boehm GC.

sdegutis2• 12 years ago

C is inherently not suitable for a GC. If you want automatic memory management, it's better to use something like Go. I'm not saying this is a bad GC library, just that I'm not sure I would recommend it for general use when writing C code.

616c• 12 years ago

This is not to troll, but Nimrod (which has been mentioned a few times on list) is a language that looks something like mainly Pascal or Python and builds C code that will be subsequently compiled with a soft-realtime garbage collector or without (it does code elimination, GC included, if you are clever). If you are nuts you can opt to force using the Boehm GC with it, but do not expect great things out of it. There is also Rust. I am sure if you spend time on HN you have heard a lot about the latter.

zurn3• 12 years ago

it usually leaks memory

Citation needed.

Integers matching valid pointers to allocated objects are rare - only common on 32-bit when running near limits of virtual memory space. Which is a bad idea anyway.

Language runtimes that eventually move off Boehm, such as Mono, usually cite other reasons.

greenyoda• 12 years ago

I'm not sure how malloc/free could be considered "advanced" C programming techniques. It's pretty hard to write any non-trivial program in C without using malloc/free and knowing the differences between static, stack-allocated and heap-allocated memory.

pfacka10• 12 years ago

The challenging part is to get details of using them correctly and avoid security and portability issues and I consider it neccesaty introduction for section dealing with memory management.

spc476• 12 years ago

And he didn't get all the details right.

    realloc(valid_pointer,0);

IS defined (in C89 no less) to act like free(). It is NOT operating system dependent.

adultSwim2• 12 years ago

You'd be surprised with how far you can get without dynamic memory or recursion...

Still, I agree with your general sentiment. That some of these are "advanced" topics is troubling. Too many programmers today don't know how a computer works. Our ideas of "mastery" are way too low.

erobbins• 12 years ago

in the modern climate of bootcamped rails hackers, knowing that memory even needs to be allocated at some point is advanced.

Iftheshoefits• 12 years ago

Depends on one's reference point. A lot of people in the web development world (for example) would probably consider manual memory management to be an advanced topic, because typically they've "grown up" with languages where memory management was so removed or abstracted as to be almost irrelevant beyond trivial "do I have enough RAM to hold this array?" type concerns.

dinkumthinkum• 12 years ago

Well honestly then they are sort of beginners to computer programming. Using malloc is not advanced C programming.

antirez• 12 years ago

Boehm is a bad advice in general... there are situations where it could work maybe, like if you implement an interpreter for the fun of it or something like that.

For many kind of programs reference counting is the way to go for C, it still is manual, but an order of magnitude safer...

freyrs3• 12 years ago

It also makes profiling much harder. Boehm doesn't play nice with valgrind without a fair bit of work.

adultSwim2• 12 years ago

Worth reading.

ghswa2• 12 years ago

I'm enjoying this so far although a couple of things have made me scratch my head in the example code allocating a vector.

  int create_vector(struct vector *vc, int size) {

    vc->data = 0;
    vc->size = 0;

    if (vc == NULL) {
      return VECTOR_NULL_ERROR;
    }

    /* check for integer and SIZE_MAX overflow */
    if (size == 0 || size > SIZE_MAX) {
      errno = ENOMEM;
      return VECTOR_SIZE_ERROR;
    }

Accessing the fields of vc before the NULL check seems like an error. Also, would it not be simpler to change type of the size parameter to size_t so that it's the same type as the size field of the vector struct?

pfacka10• 12 years ago

Yep, that is definitely error

vinkelhake7• 12 years ago

Yes, by having the NULL check after those members have been accessed, you're telling the compiler that vc cannot be NULL (because accessing the members if vc is NULL would be undefined behavior). The compiler can (and GCC will) eliminate the NULL check completely.

ghswa2• 12 years ago

Does gcc make any kind of assumptions about signed-overflow? For example, would it eliminate the comparison in this code:

  int8_t i;

  /* ... */

  i += 1;

  if (i == 0) {
    /* ... */
  }

silentbicycle2• 12 years ago

Would you mind elaborating on that? I don't see this behavior with GCC 4.2.1 on OSX or OpenBSD at -O3 when I look at the disassembly.

signa112• 12 years ago

huh :) have you actually tried doing that i.e. call the function with NULL params and see ? you will see a segfault. anyways, rather than rely on vagaries of -O2 implementation on compilers, i would rather be explicit about it and just assert invariants...

fit2rule• 12 years ago

Relying on the compiler is not the way to produce safe code.

pfacka10• 12 years ago

Thanks, for pointing out

vinkelhake7• 12 years ago

You suggest using 'calloc' to avoid calculating the size of the allocation (which would be necessary with 'malloc'). This is fine, and you store the number of elements in the 'size' field. But later, in the grow function, you double 'size' and then use it as-is in the call to 'realloc' which expects size to be in bytes, not elements.

eliteraspberrie2• 12 years ago

It is also impossible for a variable of type int to have a value greater than SIZE_MAX.

scatters• 12 years ago

On conventional platforms, sure, but there's nothing in the standard stopping int being larger than size_t. It would make plenty of sense on segmented or small-pointer architectures, like x32 (word size 64 bit, pointer size 32 bit).

exDM692• 12 years ago

Accessing the fields of vc before the NULL check seems like an error. Also, would it not be simpler to change type of the size parameter to size_t so that it's the same type as the size field of the vector struct?

What should be done in case vc is NULL? Currently it almost certainly causes a segfault, but I've seen people insist on returning an error code when a NULL argument is given but that might go unchecked. A segfault is easy to debug, a forgotten error code is not.

    int poke_foo(struct foo *foo) {
        // if you're writing a library, this is asking for trouble...
        if(foo == NULL)
            return -1; // eventually someone forgets to check for this
        foo->bar += 1;
        return 0;
    }

The only sensible thing you can do is add an assert. That will stop the program when the error happens and make it easy to debug.

    int poke_foo(struct foo *foo) {
        assert(foo != NULL); // asseting here...
        foo->bar += 1; // ... is not much better than segfaulting here
        return 0;
    }

But in the end, we're still reliably crashing the program if NULL is passed and it is as easy to debug as a segfault would. The assert may keep your static analysis tool (like Coverity) happy but it won't really make a huge difference.

NULL pointers are generally not very difficult to debug, what is hard is dealing with free()'d pointers and other non-null invalid pointer values.

pjc502• 12 years ago

"easy to debug as a segfault would"

I'm on a weird embedded platform where segfaults are almost impossible to debug; the only way to get information out is a log facility over USB that loses the most recent lines of log when it segfaults and resets :(

jibsen• 12 years ago

Writing a dynamic array in C is a good exercise in the amount of details you are spared in languages which provide basic containers.

I recently revisited some example code I wrote many years ago [1], and found some new issues, and there are most likely still some I haven't noticed.

That being said, when you use a title like this article's, it is a good idea to check your example code thoroughly.

[1] https://github.com/jibsen/scv

bstamour• 12 years ago

Is the phrase "locator value" defined by the C standard? I've never heard of it before. I always knew the 'l' in "lvalue" as meaning "this expression can appear on the left-hand side of an assignment operation."

EDIT: Found the answer (thanks, draft C11 standard.)

Section 6.3.2.1 contains the definition of lvalue, and it does not use the phrase "locator value" at all. However if you want to use it as a reminder that modifiable lvalues can be assigned to, then more power to ya :-)

I can't help it - I'm a sucker for standardese.

deletes• 12 years ago

How does this check for anything??:

   if ( size == 0 || size > SIZE_MAX ) {...

size is not given a type, but if it is a size_t, then the comparison is constant and if statements is always false, unless size == 0. The second part is useless, since the maximum size of size_t == SIZE_MAX.

jwise0• 12 years ago

Worse, it is not even a certain check against integer overflow. A particularly large expression could have overflowed substantially enough that it becomes positive again!

This, in fact, was the source of a vulnerability in PHP [1]. (The way they 'resolved' it, uh, isn't much better.)

There is almost never a magic bullet for integer overflow. Programming secure systems requires thought.

[1] http://use.perl.org/use.perl.org/_Aristotle/journal/33448.ht...

comex• 12 years ago

Note that since it says 'size == 0', it's not merely "not a certain check", but a check that passes for almost all overflowing values :)

tterrace• 12 years ago

Hmmm, that looks oddly familiar: http://use.perl.org/use.perl.org/_Aristotle/journal/33448.ht...

pfacka10• 12 years ago

It seems that I have totally messed up whole vector example by last minute changes. The check was meant to illustrate what eliteraspberrie described in previous comment.

bch• 12 years ago

The article says two different things about how free() works:

  1. In case of NULL pointer free does no action.
  2. In "double free corruption" section, it says "Could be caused by calling free with pointer, which is [..] NULL pointer"

So: which is it? Otherwise, there's no point in the NULL/assert() dance, you can freely free() with impunity:

  struct foo *a, *b, *c;
  a=NULL; b=NULL; c=NULL;
  a=malloc(sizeof *a);
  b=malloc(sizeof *b);
  c=malloc(sizeof *c);

  if(!(a && b && c)) {free(a); free(b); free(c); return 1;}

yan• 12 years ago

What the article intended to say is calling free() on the same pointer twice is considered 'double free'. Also an issue with the c++ delete operator.

Calling free() on NULL is a no-op.

EpicEng3• 12 years ago

Both. free(null) is fine. free(something_already_freed_or_not_returned_by_malloc) (and friends) is undefined behavior.

eliteraspberrie2• 12 years ago

Good advice. Integer arithmetic is one of the trickiest aspects of C, and dangerous in combination with the manual memory management. For more information, see the free chapter of TAOSSA: http://pentest.cryptocity.net/files/code_analysis/Dowd_ch06....

There are (were) a couple bugs in the example code. Here are a some guidelines that will help avoid those, and most problems with integer operations and the heap in general.

First, don't mix unsigned and signed types in arithmetic; and always prefer the size_t type for variables representing the size of an object.

Second, check for overflow before an operation, not after, like so:

    if (size > SIZE_MAX / 2) {
        goto error;
    }
    newsize = size * 2;

Third, always double-check the arguments to memory allocation functions, especially for zero, because the result is not always well defined.

    if (size >= SIZE_MAX - n) {
        goto error;
    }
    foo = malloc(size + n);
    foo[size] = ...;

sirclueless• 12 years ago

In particular, the code snippet from the blog post,

    if (size && size > SIZE_MAX) {
        errno = ENOMEM;
        err(1, "overflow");
    }

is a total no-op. It can't ever be true, the compiler might as well just remove the whole thing.

nkurz• 12 years ago

The chapter sounded interesting, but your link didn't work. Here's a fixed up version: http://pentest.cryptocity.net/files/code_analysis/Dowd_ch06....

(Not your fault. I too miss the good-old-days when copying a PDF link from Google didn't involve multiple steps or URL decoding.)

cjensen• 12 years ago

Wow. That's full of falsehoods like these:

"What happens is that variable i is converted to unsigned integer." No: 'long i' is converted to 'unsigned long'.

"Usually size_t corresponds with long of given architecture." No: For example, on Win64 size_t is 64 bits whereas long is 32 bits.

If you're going to write about Advanced Programming, you should be careful to actually be correct.

ahy1• 12 years ago

Wow. That's full of falsehoods like these:
"What happens is that variable i is converted to unsigned integer." No: 'long i' is converted to 'unsigned long'.

Actually, unsigned long is an unsigned integer. He didn't write unsigned int.

"Usually size_t corresponds with long of given architecture." No: For example, on Win64 size_t is 64 bits whereas long is 32 bits.

"Usually" is the keyword here. He could have said "Usually size_t has at least the same amount of bits as long" and it would be better related to the referred rule.

zurn3• 12 years ago

The "short" long is fantastically tasteless, it's not like source compatiblity with Win16 is has resulted in 64-bit builds of apps appearing.

pfacka10• 12 years ago

I agree that wording is bit unclear. I will try to find better one, thanks.

Scene_Cast22• 12 years ago

The sum() example does not work as the article says it does. Under Visual Studio 2013 compiling for x86 (C++ compiler, but C++ also has integer promotion rules), the function returns zero. The reason is: 65535 = 2^16-1. uint16_t is signed, therefore it has 15 bits to represent the value. When executing "int16_t a = 65535;" under a debugger, "a" is set to -1.

EpicEng3• 12 years ago

It's actually undefined behavior.

pfacka10• 12 years ago

Thanks, I tested with GCC and Clang on Linux on 64-bit x86, GCC+OpenBSD on 32-bit+x86 and GCC+Linux on PowerPC 603. The point I was aiming for is that described operation is indeed undefined.

burstmode• 12 years ago

If uint16_t really is a signed 15bit value under VS2013, somebody in the compiler development department has a great sense of humor.

Scene_Cast22• 12 years ago

My bad - that's a typo. I meant int16_t, the same as in the sample code.

arunc• 12 years ago

Wondering why C++ style has been followed, at least, for the * association with the type and not with the variable.

rquirk• 12 years ago

What about that funky braces style? Code with wrong braces or whitespace messups indicate a lack of care. If the coder can't put a { in the right spot, what else has he screwed up?

zurn3• 12 years ago

+1 for promoting Boehm GC.

GCC uses it.

edit: also Inkscape, w3m.

jheriko2• 12 years ago

"Check for NULL at beginning of function or blocks which are dereferencing pointers to dynamically allocated memory"

I really strongly disagree with this. Better for it to crash if you expect this memory to always have been allocated. This way you can fix your bug instead of putting your app into some potentially unexpected state... if allocations fail it doesn't make sense to just 'carry on anyway' in many situations.

pfacka10• 12 years ago

Noted, thanks, failed allocation is definitely good candidate for fail fast scenario.

dkersten2• 12 years ago

I disagree. Unless you can know for absolute certainty that there is no cleanup required, it is not a good idea to simply crash and leave resources in an inconsistent state.

Otherwise, you need a way for the cleanup code to dispose of the resources and leave the system in a consistent state before crashing.

Ok, its obviously good practice to develop your software in a way that a random outage doesn't leave anything in an inconsistent state, but a lot of software fails to do this. At the very least, you may end up with something like when a server crashes and cannot restart because the port is still considered in use by the OS.

jheriko2• 12 years ago

if you are writing software which has to have some kind of guaranteed reliability then maybe there is a sort of argument for this - but unless you are handling the failure case in a deterministic and well behaved way then I still think that is a much worse state to be in than a crash. Even in production. In either case you are putting the system in a potentially weird or unrecoverable state - the difference with a hard crash is that you know straight away and its easier to debug, even after its shipped.

Imagine the customer error report or imagine its a critical system and it starts making mistakes...

For most software smoke testing heavily is enough to make it super rock solid. I know that most software does not do this - just using a web browser or a smartphone makes it painfully obvious that even the big software houses have some seriously shoddy practices and that testing gets seriously neglected (it may be that its impractically big... i stuggle to buy that tbh)

aktau• 12 years ago

Just a small question:

__m128i c = _mm_set1_epi16(2) __attribute__((aligned(16)));

Don't gcc/clang take care of aligning that type automatically? Isn't the attribute thus redundant?

Guide to Advanced Programming in C

Related Stories