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.
Thanks!
> 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
functions.
* subversion/libsvn_wc/util.c
(svn_wc_conflict_description_create_text),
(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_text),
(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 :)
Thanks,
Stefan
---------------------------------------------------------------------
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