Re: cvs commit: squid3/src String.cc

On Thu, 2008-01-24 at 10:55 +1300, Amos Jeffries wrote:
> > On Thu, 2008-01-24 at 09:57 +1300, Amos Jeffries wrote:
> >> >
> >> > On Wed, 2008-01-23 at 19:07 +0200, Tsantilas Christos wrote:
> >> >> squid crashes after this patch, something is wrong. After reverting
> >> to
> >> >> a previews version of String.cc all are OK again .....
> >> >>
> >> >> Amos Jeffries wrote:
> >> >> > Reduce call duplication in String.
> >> >
> >> > I committed a fix to HEAD. However, I see other suspicious behavior
> >> when
> >> > doing ICAP so there may be more problems to fix.
> >>
> >> Definately. I'm not finished there.
> >> This was just one of the validation failures in String. Thanks for
> >> finding the caller. I've also beefed up limitInit itself to handle the simple
> >> 0-len case it should be able to cope with by itself.
> >
> > I did find more problems caused by the "call reduction" changes and will
> > commit a fix soon. I am going to change a few names and make a few calls
> > private to avoid this kind of confusion in the future.
>
> FYI, I'm working toward a single limitInit(str,len), clean()/reset(), and
> constructors.
>
> >
> > Amos, you may feel like you were cursed by SquidString, but the reality
> > is that the core API problems were introduced or not removed much
> > earlier, when C strings were half-converted to C++. You are just being
> > bitten by the code that is very difficult (and, hence, dangerous) to
> > modify, even a little.
>
> Sigh. I'm finding that. Thus the miniscule changes with major input
> validation tests.

When the code is both ugly and difficult to modify correctly, minuscule
changes are as dangerous as a rewrite, but far less productive (as this
thread indicates).

I have committed a polished version with a few TODO items that you might
want to pursue. Besides fixing the bug introduced by the recent
minuscule changes, the primary goal was to separate low-level private
methods from public ones. The secondary goal was to document and polish
the code. The third goal was to reduce the number of double-clean()
calls (just to make you happy -- it does not really buy us much :-).

The code can be further improved, of course, but you should notice a
cleaner and more consistent use of basic methods inside String.cc. There
were no changes outside String.cc and SquidString.h

HTH,

Alex.
Received on Wed Jan 23 2008 - 16:11:14 MST