You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2010/03/17 23:14:01 UTC

Re: svn commit: r922867 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/commit.c libsvn_wc/lock.c

On Sun, Mar 14, 2010 at 10:36,  <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/lock.c Sun Mar 14 14:36:26 2010
> @@ -1789,8 +1789,19 @@ svn_wc__acquire_write_lock(const char **
>       SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, child_abspath, FALSE,
>                                    iterpool));
>       if (kind == svn_wc__db_kind_dir)
> -        SVN_ERR(svn_wc__acquire_write_lock(NULL, wc_ctx, child_abspath,
> -                                           NULL, iterpool));
> +        {
> +          err = svn_wc__acquire_write_lock(NULL, wc_ctx, child_abspath, NULL,
> +                                           iterpool);
> +          if (err && err->apr_err == SVN_ERR_WC_LOCKED)
> +            {
> +              svn_error_t *err2 = svn_wc__release_write_lock(wc_ctx,
> +                                                             child_abspath,
> +                                                             iterpool);
> +              if (err2)
> +                svn_error_compose(err, err2);
> +              return svn_error_return(err);
> +            }
> +        }

Lately, we have not been using svn_error_compose(), favoring
svn_error_compose_create() since it Does The Right Thing with the
args, whether they're errors or not. Thus, the above code would
typically be written:

  svn_error_t *err2 = svn_wc__release_write_lock(...);

  return svn_error_return(svn_err_compose_create(err, err2));


Seeing the use of svn_error_compose() is a bit jarring :-P

Cheers,
-g

Re: svn commit: r922867 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/commit.c libsvn_wc/lock.c

Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:

> Lately, we have not been using svn_error_compose(), favoring
> svn_error_compose_create() since it Does The Right Thing with the
> args, whether they're errors or not. Thus, the above code would
> typically be written:
>
>   svn_error_t *err2 = svn_wc__release_write_lock(...);
>
>   return svn_error_return(svn_err_compose_create(err, err2));

I can't use that since r922926 put the compose into a loop, but I
suppose I could change the compose into a compose_create.  In a single
db the recursive lock will be a single row so the code can simply be
removed.

-- 
Philip

Re: svn commit: r922867 - use of svn_error_compose()

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 18, 2010 at 11:30, Julian Foad <ju...@wandisco.com> wrote:
> On Wed, 2010-03-17, Greg Stein wrote:
>> > +          if (err && err->apr_err == SVN_ERR_WC_LOCKED)
>> > +            {
>> > +              svn_error_t *err2 = svn_wc__release_write_lock(wc_ctx,
>> > +                                                             child_abspath,
>> > +                                                             iterpool);
>> > +              if (err2)
>> > +                svn_error_compose(err, err2);
>> > +              return svn_error_return(err);
>> > +            }
>> > +        }
>>
>> Lately, we have not been using svn_error_compose(), favoring
>> svn_error_compose_create() since it Does The Right Thing with the
>> args, whether they're errors or not. Thus, the above code would
>> typically be written:
>>
>>   svn_error_t *err2 = svn_wc__release_write_lock(...);
>>
>>   return svn_error_return(svn_err_compose_create(err, err2));
>>
>>
>> Seeing the use of svn_error_compose() is a bit jarring :-P
>
> Any reason not to extend svn_error_compose() to accept a NULL second
> arg?

That would turn it into a noop, but that's fine.

IMO, we just shouldn't use it, but for a little extra safety it can
test for NULL.

Cheers,
-g

Re: svn commit: r922867 - use of svn_error_compose()

Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-03-17, Greg Stein wrote:
> > +          if (err && err->apr_err == SVN_ERR_WC_LOCKED)
> > +            {
> > +              svn_error_t *err2 = svn_wc__release_write_lock(wc_ctx,
> > +                                                             child_abspath,
> > +                                                             iterpool);
> > +              if (err2)
> > +                svn_error_compose(err, err2);
> > +              return svn_error_return(err);
> > +            }
> > +        }
> 
> Lately, we have not been using svn_error_compose(), favoring
> svn_error_compose_create() since it Does The Right Thing with the
> args, whether they're errors or not. Thus, the above code would
> typically be written:
> 
>   svn_error_t *err2 = svn_wc__release_write_lock(...);
> 
>   return svn_error_return(svn_err_compose_create(err, err2));
> 
> 
> Seeing the use of svn_error_compose() is a bit jarring :-P

Any reason not to extend svn_error_compose() to accept a NULL second
arg?

- Julian