On Fri, 2010-08-20, Daniel Shahaf wrote:
> I arrived at these docstrings by reverse-engineering the implementation
> and some guesses. Could someone confirm my understanding?
>
> [[[
> * subversion/libsvn_ra_neon/util.c
> (multistatus_elements, multistatus_nesting_table, validate_element):
> Add docstrings.
> ]]]
>
> [[[
> Index: subversion/libsvn_ra_neon/util.c
> ===================================================================
> --- subversion/libsvn_ra_neon/util.c (revision 987515)
> +++ subversion/libsvn_ra_neon/util.c (working copy)
> @@ -82,6 +82,7 @@
> * for our custom error parser - could use the ne_basic.h interfaces.
> */
>
> +/* List of XML elements expected in 207 Multi-Status responses. */
> static const svn_ra_neon__xml_elm_t multistatus_elements[] =
> { { "DAV:", "multistatus", ELEM_multistatus, 0 },
> { "DAV:", "response", ELEM_response, 0 },
> @@ -99,6 +100,14 @@
> };
>
>
> +/* Sparse matrix listing the permitted child elements of each element.
> +
> + The permitted direct children of the element named in the first column are
> + the elements named in the remainder of the row.
> +
> + There may be any number of rows, and any number of columns in each row; any
> + non-positive value (such as SVN_RA_NEON__XML_INVALID) serves as a sentinel.
> + */
> static const int multistatus_nesting_table[][5] =
> { { ELEM_root, ELEM_multistatus, SVN_RA_NEON__XML_INVALID },
> { ELEM_multistatus, ELEM_response, ELEM_responsedescription,
> @@ -115,6 +124,9 @@
> };
>
>
> +/* PARENT and CHILD are enum values of ELEM_* type.
> + Return a positive integer if CHILD is a valid direct child of PARENT,
> + and a negative integer otherwise. */
>From my own peering at it, there's a bit more to it: if not a valid
child, it returns specifically the status listed in the table for that
parent, INVALID for some parent elements and DECLINE for others. I
assume that is important.
> static int
> validate_element(int parent, int child)
> {
> ]]]
I notice that there is a type 'svn_ra_neon__xml_elmid', typedef'd to
'int', which is used in some places (but far from all places) for enum
values of ELEM_* type. Whether 'svn_ra_neon__xml_elmid' is intended to
be used for non-element-id status values (INVALID, DECLINE) as well, I
have no idea.
The actual argument passed as 'child' is of that data type. Changing
the function prototype to use that type instead of 'int' for 'child'
would give valuable information. Tracing the origin of 'parent', I
*think* logically it has to be of that type too - in other words a valid
element enum value rather than a status code, but it arrives as 'int'.
But it's probably not a good idea to change the data type in just one
place; if anyone is going to do that they should figure out the overall
pattern and do it consistently.
Also I can deduce that start_207_element(), which calls
validate_element(), implements svn_ra_neon__startelm_cb_t. I'll state
that in a doc string because it usefully leads to documentation about
its inputs and outputs. Similarly for end_207_element().
And *THANK YOU*.
- Julian
Received on 2010-08-20 19:13:29 CEST