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

Re: [PATCH] Re: svn commit: r1476563 - /subversion/trunk/subversion/svnserve/serve.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Sat, 27 Apr 2013 17:51:22 +0100 (BST)

Daniel Shahaf <danielsh_at_elego.de>
> Bert Huijben wrote:
> Daniel Shahaf wrote on Sat, Apr 27, 2013 at 17:46:42 +0300:
>> Bert Huijben wrote on Sat, Apr 27, 2013 at 16:43:29 +0200:
>>> (Another option would be to start using our tristate enum for this case...
>>> But we made this svnserve<->libsvn_ra_svn api public following our old
>>> inter-library rules)
>>
>> +1

It may be a good idea to make a straight fix/reversion of the recent commit first, and then apply this change in a separate commit.

Some minor comments on the patch...

> [[[
> Allow ra_svn protocol parsing to use svn_tristate_t instead of
> a non-robust combination of TRUE, FALSE, and a custom macro.
>
> This commit reverts r1476563, which introduced a bug (see r1466659).
>
> * subversion/include/private/svn_ra_svn_private.h
>   (svn_ra_svn__parse_tuple): Add '3' format code, like 'B'.
>
> * subversion/libsvn_ra_svn/marshal.c
>   (svn_ra_svn__parse_tuple): Ditto.
>
> * subversion/svnserve/serve.c
>   (update, switch_cmd): Use svn_tristate_t, revert r1476563.
> ]]]
>
> [[[
> Index: subversion/include/private/svn_ra_svn_private.h
> ===================================================================
> --- subversion/include/private/svn_ra_svn_private.h    (revision 1476596)
> +++ subversion/include/private/svn_ra_svn_private.h    (working copy)
> @@ -186,6 +186,7 @@ svn_ra_svn__skip_leading_garbage(svn_ra_svn_conn_t
>       w     const char **          Word
>       b     svn_boolean_t *        Word ("true" or "false")
>       B     apr_uint64_t *         Word ("true" or "false")
> +     3     svn_tristate_t *       Word ("true" or "false")
>       l     apr_array_header_t **  List
>       (                            Begin tuple
>       )                            End tuple
> @@ -197,7 +198,10 @@ svn_ra_svn__skip_leading_garbage(svn_ra_svn_conn_t
>   * contains two elements, an error will result.
>   *
>   * 'B' is similar to 'b', but may be used in the optional tuple
> specification.
> - * It returns TRUE, FALSE, or SVN_RA_SVN_UNSPECIFIED_NUMBER.
> + * It returns TRUE, FALSE, or SVN_RA_SVN_UNSPECIFIED_NUMBER.  This format
> + * code is deprecated; new code should use '3' instead.
> + *
> + * '3' is like 'B', mutatis mutandis.

Inaccurate. Try:

  * '3' is similar to 'b', but may be used in the optional tuple specification.
  * It returns svn_tristate_true, svn_tristate_false or svn_tristate_unknown.
  *
  * 'B' is deprecated.  It is similar to '3', but it returns
  * TRUE, FALSE, or SVN_RA_SVN_UNSPECIFIED_NUMBER.

Also the final sentence should be changed:

  * will be set to @c NULL.  'b' may not appear inside an optional
- * tuple specification; use 'B' instead.
+ * tuple specification;  use '3' instead.

The rest looks good.

- Julian
Received on 2013-04-27 18:52:16 CEST

This is an archived mail posted to the Subversion Dev mailing list.