Patch for code cleanup in str_util.C
Der Meister
as announced in the irc-channel yesterday here is a patch which mainly
tries to clean up the code in str_util.C. The main things that were changed:
* Removed some potential buffer overflows (There may still be some left)
* Made some parts of the code more readable (e.g. by using std::string
instead of char-arrays)
* Improved performance of strip_whitespaces functions (both were really
horrible - O(n^2) instead of O(n))
There were also some slight changes to the interface of some functions.
Therefore the patch changes some more files if necessary but most
changes go to str_util.C/str_util.h.
I hope this patch is helpfull and I'm looking forward to some comments.
Greetings,
Der Meister
David Barnard
> as announced in the irc-channel yesterday here is a patch which mainly
> tries to clean up the code in str_util.C. The main things that were changed:
> * Removed some potential buffer overflows (There may still be some left)
> * Made some parts of the code more readable (e.g. by using std::string
> instead of char-arrays)
> * Improved performance of strip_whitespaces functions (both were really
> horrible - O(n^2) instead of O(n))
>
> There were also some slight changes to the interface of some functions.
> Therefore the patch changes some more files if necessary but most
> changes go to str_util.C/str_util.h.
Thanks, Der Meister. This is exactly the kind of thing we would like
to do. To do it safely, we would very much like to develop a testing
framework, too. Nicolas has written a couple of tests already, but if
we are going to make comprehensive changes to the API code, then I
would like to have the tests in place first.
This will prevent problems with functions not working the same way any
more, and help us find bugs without searching for them. For example,
in your patch, I think strip_whitespace(char *str) no longer works
correctly. An automated test would catch this instantly.
The third thing we would like to do is create documentation for the
API. I'm sure you have noticed oddities like strip_whitespace()
actually trims the ends, and doesn't strip all whitespace. We have
decided to use Doxygen to put the documentation right next to the
code.
If you create a separate patch with just the overflow fixes, we can
probably apply that immediately. But drop back in to #synecdoche and
we will talk more.
Regards
David Barnard
Der Meister
> This will prevent problems with functions not working the same way any
> more, and help us find bugs without searching for them. For example,
> in your patch, I think strip_whitespace(char *str) no longer works
> correctly. An automated test would catch this instantly.
However, I've tested this function and I think that my version of
strip_whitespaces(char* str) works.
> The third thing we would like to do is create documentation for the
> API. I'm sure you have noticed oddities like strip_whitespace()
> actually trims the ends, and doesn't strip all whitespace. We have
> decided to use Doxygen to put the documentation right next to the
> code.
That sound's like a good idea. Should my patch contain such
documentation for the functions I've changed or should I wait until
we've discussed and agreed on a common style for writing these comments?
Or do such style guidlines already exist?
> If you create a separate patch with just the overflow fixes, we can
> probably apply that immediately. But drop back in to #synecdoche and
> we will talk more.
OK, I'll do that as soon as I find time for it.
Greetings,
Der Meister
David Barnard
> That sound's like a good idea. Should my patch contain such documentation
> for the functions I've changed or should I wait until we've discussed and
> agreed on a common style for writing these comments? Or do such style
> guidlines already exist?
We have discussed some documentation guidelines, but (ironically) they
aren't documented yet. I'll do it today. Our plan is to add
documentation opportunistically. If we spend half a day working out
how something works, we might as well record it. We don't have the
resources yet to devote time exclusively to documentation, but I'm
hoping that by the time we have fixed the biggest problems, we will
have some worthwhile documentation to work with.
New code should always have documentation.
Regards
David Barnard