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

Re: [PATCH] Initialize commit Info to NULL

From: David James <james_at_cs.toronto.edu>
Date: 2006-11-13 09:40:46 CET

Hi Bhuvan,

I've attached a few comments below.

On 10/26/06, Bhuvaneswaran <bhuvan@collab.net> wrote:
> - return SVN_NO_ERROR;
> + {
> + commit_info_p = NULL;
> + return SVN_NO_ERROR;
> + }

This looks like a typo. It should be *commit_info_p = NULL. Otherwise,
this part of your patch looks good.

> static svn_error_t *
> -remove_tmpfiles(apr_hash_t *tempfiles,
> +remove_tmpfiles(svn_commit_info_t **commit_info_p,
> + apr_hash_t *tempfiles,
> apr_pool_t *pool)
> [...]
> /* Split if there's nothing to be done. */
> if (! tempfiles)
> - return SVN_NO_ERROR;
> + {
> + commit_info_p = NULL;
> + return SVN_NO_ERROR;
> + }

This doesn't look right. If the commit produced no tempfiles, does
that mean that there are no commitables, and that commit_info_p was
not properly set? I don't see why that would be the case.

> /* Early release (for good behavior). */
> if (! (commit_err || unlock_err || bump_err || cleanup_err))
> - return SVN_NO_ERROR;
> + {
> + commit_info_p = NULL;
> + return SVN_NO_ERROR;
> + }

This doesn't look right either. If no errors occurred, then
commit_info_p should contain a set of valid committables. It's
incorrect to set commit_info_p to NULL here.

> -/* We have a long standing bug where svn_client_commit_info_t and
> - svn_commit_info_t output parameters are not properly initialized to NULL
> - when there is nothing to commit. This is a workaround. Yes, it is
> - unspeakably evil that this was not just fixed at source when it was
> - discovered initially. */
> -%typemap(in, numinputs=0) BAD_OUTPUT_INIT_HACK ** ($*1_ltype temp = NULL)
> - "$1 = &temp;";
> -%apply BAD_OUTPUT_INIT_HACK ** {
> - svn_commit_info_t **,
> - svn_client_commit_info_t **
> -};
> -
> [...]
> svn_wc_status2_t **,
> - void **set_locks_baton
> + void **set_locks_baton,
> + svn_commit_info_t **,
> + svn_client_commit_info_t **

Nice! Is this patch to svn_types.swg safe to apply, in combination
with your little fix to svn_client_import2? (Are there other places
that still need to be patched, or is that all we need?)

Cheers,

David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 13 09:41:19 2006

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.