If you look at where they are used, then it looks like these macros do not need to compare m[1] to c1 because whenever they are used it is already known that m[1] == 'O' (hence "str3O" in the name -- that's 3 and the letter O, not number zero).
I guess c1 is included in the first case because it's faster to compare the whole 32-bit word than mask out the known byte (and since c0, c1, c2 and c3 are compile time constants, the whole right-hand-side of the comparison will be optimized by constant folding anyway).
(It's ridiculous micro-optimization in my opinion, but at least it is correct, and it is limited to a single file.)
Mmm, I would say it's definitely formatted beautifully. But I agree with the poster above in saying that it's error prone.
I'm a big Nginx fan, but I think it's good by just sheer force of will... if you had like 10 people working on it it would probably fall apart. Something sqlite at least has extensive automated tests, which makes me much more confident about its quality.
I spend most of my time in dynamic languages so I have to ask... is this that much faster than a good regex library that it warrants a hand rolled state machine? How normal is something like this in a typical C/C++ codebase?
A "good" regex library will dynamically generate code; and if the regex is simple enough, generate code that implements a DFA rather than NFA (or PDA, for Perl regexes). So for a "good" library, no, it won't be much faster.
But very few regex libraries are that "good", because it's a combination of extreme speed with very low flexibility (DFA has exponential state explosion in worst cases compared to equivalent NFA, and isn't able to deal with e.g. backreferences). The vast majority of good regex libraries will be an order of magnitude slower. Average regex libraries included in most language distributions will be slower again.
The chief exception is compiler lexer generators like lex and flex. They produce code very similar to the state machine linked. And that's probably the most common place to see this kind of thing.
Encoding the state of the machine implicitly as the program counter, rather than an explicit state variable, often results in more readable code and is the more usual way to do it when writing it by hand. It also saves a register, important on some architectures. But the technique has slightly more limited expressiveness owing to needing to stick with structured programming constructs.
People (should) only implement things in C/C++ after thoroughly profiling the dynamic language POC and being sure they need the performance that a C implementation affords. So yes, the C implementation is going to sacrifice readability for performance. Of course, there are better and worse ways of accomplishing the same thing.
I understand that they're doing string comparisons (of a kind), but what are those values defined in 'uint32_t'? Is it a translation of values into memory addresses?
I haven't given this more than a cursory look, but I love how the body message for the commit that seemingly introduced this flaw is "No functional changes.": https://github.com/git-mirror/nginx/commit/8a3f6ce
* Has functional change + introduces bugs
* No functional changes + introduces bugs
* Has functional changes + doesn't introduce bugs
* No functional changes + doesn't introduce bugs
Also I haven't seen too many commit that say "this introduces 5 new accidental bugs into the system". Just saying, not sure what ou are criticizing here
Maybe I'm getting it wrong, but this comment seems so arrogant.. and with ngninx, that has one of the cleanest code bases of open source software actually used by many people :s
To me "no functional changes" just means that the commit does not mean to introduce/change any functionality, for example if you refactor code or clean up.
It doesn't look arrogant to me. And it doesn't matter how clean the code looks like if it uses trillions of mutable states, lots of inconsistent interfaces, occasional pointer arithmetics, manual memory management, gotos all over the place and no module tests whatsoever. I'd say nginx is a pretty error prone piece of software, just like most of the software written in C.
I didn't mean nginx parser and state machines in general as mutable states, but rather state changes in variables across multiple functions and files. They are impossible to keep track of and nginx is full of them.
Even though I told the installer to keep the default config (which it did) but over-written /etc/nginx/conf.d/default.conf The only change I had to make was port number, since I am using varnish changed my port to 8080 for nginx. Restarted nginx.
Another note: Packages provided by Ubuntu use /etc/nginx/sites-available/ and /etc/nginx/sites-available/ for storing site configs but the package directly from Nginx stores configs in /etc/nginx/conf.d/.
Not sure what the schedule looks like in general, but last night I waited ~10 minutes after the advisory watching the packages show up one by one. Still faster than any other source, of course.
In this case goto is used as a form of "exception" handling, which is a perfectly fine use case for goto if you have an imperative language like C without exceptions or exception handling mechanisms. In fact it would probably be bad style not to use goto in this case.
If you have a better way to express the control flow you should use it instead though.
It's not uncommon in C as an alternative to exceptions. You don't want to be using it for every-day flow control, but when you're checking results from syscalls repeatedly, using a gigantic nested if-else chain would get more messy than using goto. The other alternative is early return/exit calls, but then you have to remember to clean up everything each time before you return/exit.
GOTO is often involved in error handling. If there are a bunch of different ways that code inside a function can fail, and the cleanup after failure involves tearing down the same objects, etc... then it's usually considered acceptable to use GOTO to target it.
1.3 was the beta stream, and 1.2.8 is the most recent 1.2 release, so in those few weeks the only stable release you've missed is 1.4.0 (which may well have not been packaged at all, if the distro was waiting a few days for bugs like this to get ironed out)
