On: Aaron Swartz's Thoughts on djb (2009)

[brotherr]

Are you people serious? His code is absolutely disgusting:

    uugh = constmap(&mapuser,x,i);
    if (!uugh) die_user(x,i);
    ++i; x += i; xlen -= i; i = byte_chr(x,xlen,':'); if (i == xlen) return;

https://github.com/amery/qmail/blob/aa6bf9739209ca76f7f3af0f...

It's mostly bug-free because it scares bugs away. If I was a bug I wouldn't want to live in there.

[cremno → brotherr]

What are your issues with it? That the variable names are not descriptive enough to be understood when the code is taken out of context? That the if-substatements aren't compound statements? The last line? What would you have done instead (back then)? Every statement on a new line makes it more difficult to understand it since it belongs together, a macro obscures the code and also has to be named, and a function only for this is (was) too expensive speed and memory wise.

[viraptor → cremno]

Not the gp, but I agree with him. I dislike everything you listed, but think that even with context it's unreadable. Context is hard to find too since this file has 3 lines of comments only. (For an enum equivalent) Also, I don't buy the "function too expensive" idea - that looks like a crazy micro optimization. If it was needed, I'd say it deserves a comment.

[cremno → viraptor]

Disliking omitting braces or really short names is just a matter of taste (however both is classic C coding style), but you can easily find out what they store without comments or a longer name. I personally would have chosen l instead of x, which is a pointer to a character in the current read _l_ine (see L202).

Can you tell me what comments you're expecting? qmail comes with a bunch of man pages (including ones for functions and they're well written). The file is called qmail-pw2u.c and there's qmail-pw2u(8). The function is called dosubuser, so let's look up subuser in it:

> Extra addresses. Each line has the form

> sub:user:pre:

That gives us a pretty good idea about the purpose of the posted code (plus the previous lines).

I agree, a function or macro makes sense (since there are many other equivalent constructs in the same file), but that's "just" giving a bunch of statement a name. Does field_next explain what they do?

   static int field_next(int *i, char **x, unsigned *xlen) {
       *++i; *x += *i; *xlen -= *i; *i = byte_chr(*x,*xlen,':');
       return i == xlen;
   }
   / * later */
   if (field_next(&i, &x, &xlen)) return;

The rest of the function is writing something. Maybe we should find out the purpose of the program at first? The man page tells us this too:

>qmail-pw2u reads a V7-format passwd file from standard input and prints a qmail-users-format assignment file.

[russtrotter → brotherr]

I can't your extent of sarcasm from the comment, but the final sentence made me cackle: "If I was a bug I wouldn't want to live in there"! ha!

[AlexandrB → brotherr]

It's extremely terse.

It's a question of priorities, seems like djb prioritizes correctness over readability - which is fine, but does limit the size of his audience.

[_glsb → brotherr]

An eloquent, objective, and thoroughly researched opinion.