[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] initialize struct members

From: Stefan Sperling <stsp_at_elego.de>
Date: Wed, 10 Sep 2008 22:47:49 +0200

On Wed, Sep 10, 2008 at 09:11:43PM +0200, Stefan Küng wrote:
> Stefan Sperling wrote:
> > On Wed, Sep 10, 2008 at 08:01:12PM +0200, Lieven Govaerts wrote:
> >> Stefan Küng wrote:
> >>> Hi,
> >>>
> >>> Before I commit this, maybe someone likes to first review it:
> >>> this patch initializes all struct members in the functions
> >>> svn_wc_conflict_description_create_text() and
> >>> svn_wc_conflict_description_create_prop().
> >>> Found this due to crashes in TSVN - it tried to access the merged_file
> >>> member which had a non-NULL value but pointed to bogus memory.
> >>>
> >>> If nobody objects, I'll commit this in about an hour (shouldn't be much
> >>> to object to IMHO).
> >>>
> >> Using apr_pcalloc has the same effect, but slightly easier for the eyes.
> >
> > Oops, good point, actually.
> >
> > Stefan (you, not me :), do you want to make a follow-up commit that
> > uses apr_pcalloc() instead?
> See r33019.


> Hope the commit message is ok.

I took a look (because you asked).

For reference, the log message:

Follow-up to r33017:

Use apr_pcalloc to initialize svn_wc_conflict_description_t factory

* subversion/libsvn_wc/util.c
  (svn_wc_conflict_description_create_prop): Instead of setting not
    relevant members to NULL, use apr_pcalloc instead of apr_palloc to get
    the memory for the structure.

The first sentence looks wrong. It's not the functions themselves
that are being initialised. Rather, the objects the functions are
allocating are initialised.

Also, the nested "instead of" looks a bit confusing to me (but
probably just me).

FWIW, I'd suggest something like:

Follow-up to r33017:

* subversion/libsvn_wc/util.c
  (svn_wc_conflict_description_create_prop): Use apr_pcalloc instead
    of apr_palloc to allocate memory. This way, we don't need to set
    any members to NULL manually.

That said, log messages are a matter of taste and writing style,
so you'd probably get entirely different suggestions from everyone
else :)


To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-10 22:48:49 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.