On Wed, Feb 08, 2006 at 09:28:34PM -0800, John Franks wrote:

> Actually, I removed the check for a NULL pointer since str
> should never be NULL.

This is what confused me: removing the check without removing the cause.
Smells like removing the safety belt while you are still at 300mph ;-)

> str is assigned by a call to
> syncpipe_getstr() which will, in practice,  never return a
> NULL pointer.  The only way to get there in the old code is
> to have the other side of the syncpipe write an invalid string length ( <= 
> 0).
>
> However, it looks as though I left out the modification to
> syncpipe_getstr() which calls error() when an invalid length
> is passed and guarantees a NULL is *never* returned...

Hmmm, I still don't get it.  If I understand your explanation and the
patch correctly, the "(null)" would be generated only when a fatal
protocol error occurs.

But actually, this is not the case.  The "(null)" strings appear in
every CHUNKSUCCESS line although everything seems to be OK.  Should this
really be caused by a protocol error, then the cause for the protocol
should be found.

Could it be that there is yet another place that could generate NULL?
And if so: wouldn't it be better to leave the check in place, just to
be sure?