Re: svn commit: r1412554 - in /subversion/trunk/subversion: include/private/svn_string_private.h libsvn_subr/string.c tests/libsvn_subr/string-test.c

> Author: brane

> Date: Thu Nov 22 14:00:48 2012
> New Revision: 1412554

> Modified: subversion/trunk/subversion/libsvn_subr/string.c
> svn_cstring__similarity(const char *stra, const char *strb,
>                         svn_membuf_t *buffer, apr_size_t *rlcs)
> {
> -  const apr_size_t lena = strlen(stra);
> -  const apr_size_t lenb = strlen(strb);
> +  const svn_string_t stringa = {stra, strlen(stra)};
> +  const svn_string_t stringb = {strb, strlen(strb)};

Unfortunately our C'89 coding standard doesn't allow us to use non-constant initializers, and there are some compilers occasionally used to build Subversion that don't accept it.

> +  return svn_string__similarity(&stringa, &stringb, buffer, rlcs);
> +}
> +
> +unsigned int
> +svn_string__similarity(const svn_string_t *stringa,
> +                      const svn_string_t *stringb,
> +                      svn_membuf_t *buffer, apr_size_t *rlcs)
> +{
> +  const char *stra = stringa->data;
> +  const char *strb = stringb->data;
> +  const apr_size_t lena = stringa->len;
> +  const apr_size_t lenb = stringb->len;
>   const apr_size_t total = lena + lenb;
>   const char *enda = stra + lena;
>   const char *endb = strb + lenb;
> @@ -1200,11 +1212,13 @@ svn_cstring__similarity(const char *stra
>     }
>
>   /* ... and the common suffix */
> -  while (stra < enda && strb < endb && *enda == *endb)
> -    {
> -      --enda; --endb;
> -      ++lcs;
> -    }
> +  if (stra < enda && strb < endb)
> +    do
> +      {
> +        --enda; --endb;
> +        ++lcs;
> +      }
> +    while (stra < enda && strb < endb && *enda == *endb);

Both "before" and "after" versions of this block appear to strip off (and count in LCS) not only the common suffix but also one non-matching character as well iff such a non-matching character remains in each string.  Is that the intention?  It looks wrong so please document it if so.

>   if (stra < enda && strb < endb)
>     {
> @@ -1254,7 +1268,7 @@ svn_cstring__similarity(const char *stra
>           }
>         }
>
> -      /* The common suffix matcher always finds the nul terminator,
> +      /* The common suffix matcher always incremnts the lcs

It incremented it *one more than the true common-suffix count* is the relevant point.  (And then only if there was still at least one character remaining in each string at that time; but that's the condition of the outer "if" guarding this block, so necessarily true here.)

>           so subtract 1 from the result. */
>       lcs += prev[slots] - 1;
>     }

- Julian
Received on 2012-11-22 17:23:06 CET