Re: CVS update: subversion/subversion/libsvn_delta vcdiff_parse.c

On Tue, Oct 03, 2000 at 05:01:02PM -0400, Greg Hudson wrote:
> Karl committed:
>
> > + (svn_txdelta_window_t *) apr_pcalloc (parser->subpool,
> > + sizeof(svn_txdelta_window_t));
>
> So, zeroing out the bytes of a pointer and assuming it will have the
> value of NULL is not correct C, although it tends to work in practice.

"tends" ?!?!!

Come on. It works *everywhere*. Pretending that it might not work somewhere
is rather delusion. It also creates a crufty implementation with little
benefit. Using apr_pcalloc() to avoid a bunch of 0/NULL assignments (and
most programs design their data structures to use 0/NULL for many states) is
an entirely common, typical, and valid thing to do. If it wasn't, then
apr_pcalloc() wouldn't even be necessary.

> (It's also not clean, in my opinion, even with integral types.) So
> any pointer fields of the allocated svn_txdelta_window_t should not be
> assumed to be NULL. If you're aware of this and don't care, that's
> okay, but I thought I'd make sure.

Style is one thing. If you want to manually set 0/NULL values for clarity or
whatever, then fine. But *don't* pretend it is because you can't write
zeroes into a structure to result in a NULL pointer.

There is absolutely no conceivable processor/architecture that we could or
will target with a non-zero NULL. We shouldn't add cruft to (ahem) "defend"
against those architectures.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/