Re: Bug in macLib

[Andrew Johnson <anj@aps.anl.gov>]

On Wednesday 31 March 2010 08:58:28 Eric Norum wrote:
> >
> > I may be missing something, but this code actually looks OK to me.  Note
> > that valend is a pointer, not a counter.  So if valend actually points
> > to the last character in the buffer then the statement
> >
> >            if ( v <= valend ) *v   = '\0';
> >
> > should be OK, right?
>
> Nope.
> Despite its name, "valend" actually points to the location just past the
> end of the buffer.

I'm convinced that the original intention was as Mark suggests, which is 
obvious if you look at the two other calls to trans() which both allocate 
buffers of size maxlen+1.  Also two of the four functions that call 
macExpandString() give it a maxlen value which is explicitly 1 less than their 
available buffer size.

Unfortunately the documentation (src/libCom/macLib/macLibREADME) doesn't say 
anything about the maxlen parameter, and this really doesn't follow the C 
standard, so I would be in favour of checking and changing the meaning of the 
maxlen argument to macExpandString() but *not* the valend pointer in trans(), 
which I believe is being used correctly in all cases.

Eric's proposed change to trans() is wrong, this code is pretty subtle and 
there are several other places in trans() which you'd have to change to match.

- Andrew
-- 
The best FOSS code is written to be read by other humans -- Harald Welte