"Ignacio J. Ortega" wrote:
>
> > perhaps i'm missing something, but it looks like the offset
> > returned by
> > getOffset() should be the offset into the byte array contained in
> > ByteChunk, which would be the start position in this case, rather than
> > the end position.
> >
>
> From the point of view of the o.a.t.m.s.ajp13, now getOffset() has the
> correct behavior, at least now for me works, prior to that change ajp13
> was unusable.., take a look in o.a.t.m.s.Ajp13.Ajp13Packet.appendString,
> There is some magic in the use of ob.setbyteOff and ob.getByteOff...as i
> understand this code.. ob.getByteOff is requiered to return the end of
> the last writed byte chunk ..perhaps a better aproach to this fix is to
> change ob.getOffset to use bb.getEnd instead os bb.getOffset..at least
> this will make the intention clearer.., in a really obscure code..
>
i see... kind of makes my head hurt :) i think you're probably right
about changing OutputBuffer.getOffset() -- i would make things clearer.
> > also, if you look at the append() and equals() methods that take
> > ByteChunk's as arguments, if getOffset() returns the end
> > position rather
> > than the start position, these methods won't work as intended. for
> > example, equals() will start comparing comparing bytes at the
> > end of the
> > ByteChunk, rather than the start of it.
> >
>
> I reviewed it, and your are right for equals..but not for append..append
> is suppoused to use the end of the buffer not the start..so in append
> getOffset is suppoused to return end not the start...
>
are you sure? here's the code i was looking at:
---- code snip ----
public void append( ByteChunk src )
throws IOException
{
append( src.getBytes(), src.getOffset(), src.getLength());
}
/** Add data to the buffer
*/
public void append( byte src[], int off, int len )
throws IOException
{
// will grow, up to limit
makeSpace( len );
// if we don't have limit: makeSpace can grow as it wants
if( limit < 0 ) {
// assert: makeSpace made enough space
System.arraycopy( src, off, buff, end, len );
end+=len;
return;
}
// if we have limit and we're below
if( len <= limit - end ) {
// makeSpace will grow the buffer to the limit,
// so we have space
System.arraycopy( src, off, buff, end, len );
end+=len;
return;
}
---- end code snip ----
it looks like when you call append(ByteChunk), the result will be an
IndexOutOfBoundsException. in fact, this code proves that:
public void testByteChunk() throws Exception {
System.out.println("testByteChunk");
byte[] b1 = new String("bytes1").getBytes();
byte[] b2 = new String("bytes2").getBytes();
ByteChunk bc1 = new ByteChunk();
ByteChunk bc2 = new ByteChunk();
bc1.setBytes(b1, 0, b1.length);
bc2.setBytes(b2, 0, b2.length);
String s1 = bc1.toString();
String s2 = bc2.toString();
System.out.println("s1 = " + s1);
System.out.println("s2 = " + s2);
bc1.append(bc2);
s1 = bc1.toString();
System.out.println("s1 = " + s1);
}
i believe the solution here would be to have append(ByteChunk) look like
this:
public void append( ByteChunk src )
throws IOException
{
append( src.getBytes(), src.getStart(), src.getLength());
}
> The better solution is to get rid of getOffset..and only use getStart or
> getEnd where needed...
>
> > just a sanity check...
> >
>
> Well done, OSS is great because of this comments..and the possibility to
> make them just-in-time..:)))
>
> > -kevin.
>
> Saludos ,
> Ignacio J. Ortega
