Re: svn commit: r38202 - trunk/subversion/libsvn_wc

Stefan Sperling <stsp_at_elego.de> writes:

> On Fri, Jun 26, 2009 at 07:37:25AM +0200, Greg Stein wrote:
>> Okee dokey, then. Seems fine, except that you used apr_pcalloc() when
>> a palloc would be just fine. You're filling every byte manually, so
>> there is no reason to zero it beforehand.
>
> As a general rule, I'd say just always use pcalloc. Everywhere.
> Who knows how the code in question will be tweaked in the future?

I'm not writing much Subversion code these days but I think that's
wrong. It makes me uneasy assuming that zero is a suitable
initialisation. It also defeats tools like valgrind which look for
UMRs.

> We've caused enough problems for e.g. TortoiseSVN in the past by
> passing objects containing uninitialised but non-NULL pointers
> out to callers of our libraries. They have then no way of checking
> the pointers for validity and end up dereferencing them, causing
> them to segfault. This can happen easily when structures are extended
> and the init code isn't updated.

I'd prefer it if such bugs are fixed by explicit initialisation.

> The small overhead of zeroing memory even if it's not necessary
> is much less of a problem. Can't the compiler even optimise it
> away if it is redundant?

Unlikely. pcalloc is in a separate library and its source is not
available when compiling the caller.

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2365645
Received on 2009-06-26 13:09:42 CEST