staikos at kde.org
Thu Dec 19 18:12:55 GMT 2002
On Thursday 19 December 2002 12:33, Andrew Stanley-Jones wrote:
> This was CC'ed to core devel a few days ago, so I thought I might as well
> follow up. While my first patch was wrong, but this brings up a very old
> topic, and maybe it could refresh some memories.
> The change to the code (strncpy->strlcpy) was simple and even though it
> "should not" do anything bad, broke the entire program making it useless.*
> Since the change was made by someone who did not attempt to understand the
> code, and then never tested the problem wasn't found till much later. I
> fixed head since I ran accross it.
> Yesterday while on irc on #kde-devel a little message poped up, "anyone
> tried kSirc in 3.1 branch recently?" Client was completly broken. Again a
> simple fix, and we got it good and early so no harm done.
> But it really bugs me when changes are done and not tested. I understand
> that when changing lots of code all over the place it would be very time
> consuming, but especially in stable branches how else do you know it was
> good? If you don't understand the code/or are unwilling to test it maybe
> email the maintainer?
I think the root of the problem lies elsewhere. If you look at the number
of problems we found (and the number of problems that probably still exist),
you will realise that four people, two working part time for free, cannot
possibly test all of that code. Emailing maintainers is easy, if they even
exist, but often leads to dead ends, no response, or very long response
times. It is the maintainer's responsibility to monitor his code for changes
too. If no-one is monitoring code in CVS, then it is effectively
unmaintained, despite what any text files might say.
The fact that these changes had to be made at all in CVS is yet another
problem. If there weren't so many unfortunate problems with the code, the
changes never would have been made in the first place. In this case, it
happened to not be a bug at all, but what is 1 false hit in a sea of
hundreds? Statistically unnoticeable.
Finally, I think there were problems with this code afterall. str*() are
used for manipulating null-terminated strings. Yes you can do other things
with them, but you can virtually write any app using only system() too. This
doesn't make it correct. mem*() are much better alternatives when you are
dealing with non-null terminated data. The changes were of course made and
this will help in the future I'm sure. A good alternative would have been a
comment which described that even though str*() were used, the null byte is
being disregarded if it exists. Unusual code is perhaps most deserving of
Of course this is no excuse to be careless with these fixes, but
nonetheless they have to be done, and they have to be done to the best of our
abilities within our constraints.
More information about the kde-core-devel