Hacker News Reader

Comment by tptacek | Hacker News Reader

Adding to what everyone else has said, this also "how not" to write socket code; for instance, the assumption that you can read a whole HTTP request "in one go" with a single large read call is false.

Also, casting function calls to (void) is nonsensical.

You can perhaps forgive the sprintf() call because, AIX. (Believe it or not, there was a time when snprintf was a portability problem). You can't forgive the log() function that doesn't explicitly bounds check its argument (though it's not exploitable in this code).

View on HN

> Also, casting function calls to (void) is nonsensical.

This is an older convention to indicate that the programmer knows the function returns a value but has chosen to ignore it. It got around the false positives generated by lint.

This code uses SIGCLD which I don't think is supported by BSD, where it is called SIGCHLD and is slightly different (someone please correct me if I'm wrong). If that's the case, then the author's assertion that it "should run unchanged on AIX, Linux®, or any other UNIX version" is incorrect - it will only run on System V varieties.

Confirmed the SIGCLD problem.

I had to change it to SIGCHLD to get it to compile on my Mac.

There is also the matter of directly stuffing the sockaddr_in with calls like inet_addr(), which has long been considered inferior to alternatives using getaddressinfo().

This also has the side effect of this implementation not supporting IPv6, among others.

So please, don't use this as anything other than an example how not to do sockets.

If you want to learn how to do it right, I can recommend the excellent "Guide to Network Programming Using Internet Sockets" by Brian "Beej Jorgensen" Hall (You should definitely buy the printed version, just to support him :)).

> Also, casting function calls to (void) is nonsensical.

Does extremely pedantic C require the results of function calls to be used?

I know the correct way to mark a variable as unused is to cast it to void, but I'm not sure if you're supposed to do that for function return values as well.

Nothing about the cast is "required". However, it's good practice because the cast explicitly acknowledges the function has a return value that we're throwing out. Looking at the following function call:

    (void)create_widget(&widget);

It's clear that `create_widget` returns some value -- it's probably an error code that tells us whether the widget creation was successful. The source code here says "I know this function returns an important value, but I'm going to disregard it here". This could be useful for debugging if you find that, for example, an error condition is being handled improperly (e.g. "oh, we're obviously throwing out the return value of this function, which we shouldn't be doing"). This function call is much less informative:

    create_widget(&widget);

It's a terrible and (I think) amateurish practice that misapprehends the point of C's type checking while adding lots of visual noise, which is why you'll virtually never see it in well-liked C code.

The list of MIME types looks weird, too. There's stuff like "image/zip" in the source code.

Reply on news.ycombinator.com