You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2014/08/03 11:10:36 UTC

Re: svn commit: r1615349 - in /subversion/trunk/subversion: include/svn_error_codes.h libsvn_subr/mutex.c

On 02.08.2014 21:13, stefan2@apache.org wrote:
>        /* Update lock counter. */
>        if (mutex->checked)
> -        --mutex->count;
> +        if (--mutex->count)
> +          return svn_error_create(SVN_ERR_INVALID_UNLOCK, NULL, 
> +                                  _("Tried to release a non-locked mutex"));

(Grumble ... unnecessary obfuscated nested if.)

This code will leave the mutex in an invalid state, which is never a
good idea. It would be a lot better to check the count before
decrementing it:

    if (mutex->checked)
      {
        if (0 >= mutex->count)
          return svn_error_create(...);
        --mutex_count;
      }


This made me look at the mutex implementation and ... let's just say
that it's less than efficient? Why do we need a lock counter that can
only have the values 0 and 1 (not counting the obvious bug in the
current code)?

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane@wandisco.com