On Wed, Sep 15, 2010 at 07:45:06AM +0100, Jeremy Henty wrote:
Just to clarify, I wasn't expecting any speed-up from this, it was just a cleanup patch. I accept the point about the dangers of fragile APIs in general, but I don't find it very compelling in this particular case. Html_process_word() is called from only one place, and it's caller has to hack its around the entirely unnecessary requirement that the argument be null-terminated. I don't see that any API is damaged just by cleaning up this particular function call.
First I have to say that I agree with Jorge. Working with non NULL-terminated buffers is extremly ugly and error prone. The reason is that when using length based variants of the standard string routines (e.g. strncmp() ...) you always have to recompute the length relative to your current position in the buffer. With null-terminated strings no matter where you are in the string it will still be properly terminated. On the other hand I'd also like to see Html_write_raw() to work on a const-buffer and even drop the requirement that it's buf parameter needs to be null-terminated (which is not addressed in your patch atm). The reason is that currently we pass a buflen and _also_ require buf to be null-terminated. That's rather confusing. I don't have a solution yet, but I envision something, where e.g. Html_write_raw() or DilloHtml::write() works with a const buf and buflen (no null-termination required), but non-terminated buffers don't leak into lower-layer functions (like Html_process_space(), or Html_process_word()). Also, as there is no pressing need we should be extremely careful and think twice before changing things. Regards, Johannes