On 08/29/2010 05:09 PM, Amos Jeffries wrote:
* FooInit() is a C version of a Foo class constructor. I would recommend
converting HttpParserInit() to a constructor since you are polishing
this code anyway, but if you leave init() in place, please call clear()
_after_ performing any initializations in init(). This approach makes a
future conversion much easier.
Um, no.
init() sets it to a state where data is available for parsing and about
to begin.
clear() sets it to a state where there is no data available and parsing
is invalid.
For parsers, it is a bad idea to equate "no data available" with
"invalid", but, at any rate, the clear() method is misnamed: it is a
standard name in STL and other libraries and never implies invalidation,
just emptying and resetting. Your clear, if it must invalidate, should
be called invalidate() or something. And calling it from init() becomes
very confusing; please avoid that if clear() is actually invalidate().
HttpParser is (ab)used in the current code by being re-used for multiple
requests and responses *without clearing previous offset memory*.
What do you mean by "offset memory"? Does not init() resets offsets and
pointers?
It has no
buffers of its own, it merely points at the callers buffer space and hold
offsets. Also, that callers buffer may at any time be re-allocated. Current
code gets around this by re-init().
Okay, for the basic fix+tests I'll remove clear() again. Leaving it and
your comments to the polish stage.
NOTE: we then cannot test for incorrect setting of supposedly unset
fields.
Alex.
