When I said "fluent in C", I meant "fluent in C", sorry... [LWN.net]

When I said "fluent in C", I meant "fluent in C", sorry...

Posted Dec 7, 2011 13:20 UTC (Wed) by khim (subscriber, #9252)
In reply to: trying to work out some problem points by johill
Parent article: Fedora security update to nginx

You're confusing "fluent in C" and "fluent in DNS" and assuming that everybody who reads that code must understand the DNS protocol with all its bits in its entirety, and completely know what each bit means.

I assume nothing of the sort. I knew very little about DNS protocol. In fact I've read C code first (it was quite simple, really), got confused by one small thing (why do we ignore top two bits totally?), and only then opened the RFC to see what exact structure we are parsing here.

It seems like you're suggesting it is equivalent to:

       switch (*(frame + 24)) {
       case 7:
...
               if (len < 27)
                       goto invalid;

               switch (*(frame + 25)) {
               case 1: {

The code you've showed includes "random numbers" which have no internal logic. It's easy to imagine different RFC where IEEE80211_MIN_ACTION_SIZE will be 15 or 30 and where WLAN_CATEGORY_HT will be equal to 10. Worse: it's hard to imagine that you implement your logic using the textual description which refer to "24th byte" or "7th category"!

But the code which started the discussion is radially different. It implements process which description uses terms like the first two bits of the octet (which is quite literally "c & 0xc0" in C) and the remaining six bits (which, again, is "c & 0x3f" in C).

If, indeed, the canonical description in your case includes 7th category and not the "WLAN HT category" (or something like that) then yes, I think it's better to keep "magical numbers" in your code: it'll be easier to understand then try to constantly convert named constants to "magic numbers" and back ("gcc -E" can be a big help here as I've noted). Except 27. I think "25 + 2" will be more readable even in this case...

(Log in to post comments)