You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2012/07/27 16:36:24 UTC
Heads-up: new typedef 'svn_merge_rangelist_t'
Heads up: I'm going to add
typedef apr_array_header_t svn_merge_rangelist_t;
There's no major impact; it doesn't change anything functionally.
The rest of the details:
The immediate reason is to improve the debugging experience: I can then make GDB display 'rangelist' values in a simple human-readable form, whereas presently the best it can do is recognize that it is an APR array and display something like "array of 2 items".
Beyond that, it makes sense from a coding abstraction point of view: it fills in the gap in this hierarchy of types (quoting from svn_mergeinfo.h):
/** Terminology for data structures that contain mergeinfo.
*
* Subversion commonly uses several data structures to represent
* mergeinfo in RAM:
*
* (a) Strings (@c svn_string_t *) containing "unparsed mergeinfo".
*
* (b) A "rangelist". An array (@c apr_array_header_t *) of non-overlapping
* merge ranges (@c svn_merge_range_t *), sorted as said by
* @c svn_sort_compare_ranges(). An empty range list is [...].
*
* (c) @c svn_mergeinfo_t, called "mergeinfo". A hash mapping merge
* source paths (@c const char *, starting with slashes) to
* non-empty rangelist arrays. A @c NULL hash is used to [...].
*
* (d) @c svn_mergeinfo_catalog_t, called a "mergeinfo catalog". A hash
* mapping paths (@c const char *) to @c svn_mergeinfo_t.
*
* Both @c svn_mergeinfo_t and @c svn_mergeinfo_catalog_t are just
* typedefs for @c apr_hash_t *; there is no static type-checking, and
* you still use standard @c apr_hash_t functions to interact with
* them. [...]
*/
typedef apr_hash_t *svn_mergeinfo_t;
typedef apr_hash_t *svn_mergeinfo_catalog_t;
The new type will be public. Because it's just a typedef, the C API will be backward-compatible and it won't change the ABI. (Can someone confirm that?)
As mentioned in that doc string, using such a typedef doesn't provide any static type-checking.
svn_merge_rangelist_t is not going to be a pointer like svn_mergeinfo_t and svn_mergeinfo_catalog_t are, since that is a poor idiom leading to inability to declare a parameter as "pointer to a const rangelist".
- Julian
Re: Heads-up: new typedef 'svn_merge_rangelist_t'
Posted by Greg Stein <gs...@gmail.com>.
Seems fine, and there should be no affect on the API/ABI. And thanks
for not including the '*' in there... that was a very poor choice for
the mergeinfo :-(
On Fri, Jul 27, 2012 at 3:24 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Actually, it'll be called "svn_rangelist_t", since we've already got lots of public "svn_rangelist_...()" functions.
>
> - Julian
>
>
>
>
> ----- Original Message -----
>> From: Julian Foad <ju...@btopenworld.com>
>> To: Subversion Development <de...@subversion.apache.org>
>> Cc:
>> Sent: Friday, 27 July 2012, 10:36
>> Subject: Heads-up: new typedef 'svn_merge_rangelist_t'
>>
>> Heads up: I'm going to add
>>
>> typedef apr_array_header_t svn_merge_rangelist_t;
>>
>> There's no major impact; it doesn't change anything functionally.
>>
>> The rest of the details:
>> The immediate reason is to improve the debugging experience: I can then make GDB
>> display 'rangelist' values in a simple human-readable form, whereas
>> presently the best it can do is recognize that it is an APR array and display
>> something like "array of 2 items".
>>
>> Beyond that, it makes sense from a coding abstraction point of view: it fills in
>> the gap in this hierarchy of types (quoting from svn_mergeinfo.h):
>>
>> /** Terminology for data structures that contain mergeinfo.
>> *
>> * Subversion commonly uses several data structures to represent
>> * mergeinfo in RAM:
>> *
>> * (a) Strings (@c svn_string_t *) containing "unparsed mergeinfo".
>> *
>> * (b) A "rangelist". An array (@c apr_array_header_t *) of
>> non-overlapping
>> * merge ranges (@c svn_merge_range_t *), sorted as said by
>> * @c svn_sort_compare_ranges(). An empty range list is [...].
>> *
>> * (c) @c svn_mergeinfo_t, called "mergeinfo". A hash mapping merge
>> * source paths (@c const char *, starting with slashes) to
>> * non-empty rangelist arrays. A @c NULL hash is used to [...].
>> *
>> * (d) @c svn_mergeinfo_catalog_t, called a "mergeinfo catalog". A
>> hash
>> * mapping paths (@c const char *) to @c svn_mergeinfo_t.
>> *
>> * Both @c svn_mergeinfo_t and @c svn_mergeinfo_catalog_t are just
>> * typedefs for @c apr_hash_t *; there is no static type-checking, and
>> * you still use standard @c apr_hash_t functions to interact with
>> * them. [...]
>> */
>>
>> typedef apr_hash_t *svn_mergeinfo_t;
>> typedef apr_hash_t *svn_mergeinfo_catalog_t;
>>
>> The new type will be public. Because it's just a typedef, the C API will be
>> backward-compatible and it won't change the ABI. (Can someone confirm
>> that?)
>>
>> As mentioned in that doc string, using such a typedef doesn't provide any
>> static type-checking.
>>
>> svn_merge_rangelist_t is not going to be a pointer like svn_mergeinfo_t and
>> svn_mergeinfo_catalog_t are, since that is a poor idiom leading to inability to
>> declare a parameter as "pointer to a const rangelist".
>>
>> - Julian
>>
Re: Heads-up: new typedef 'svn_merge_rangelist_t'
Posted by Julian Foad <ju...@btopenworld.com>.
Actually, it'll be called "svn_rangelist_t", since we've already got lots of public "svn_rangelist_...()" functions.
- Julian
----- Original Message -----
> From: Julian Foad <ju...@btopenworld.com>
> To: Subversion Development <de...@subversion.apache.org>
> Cc:
> Sent: Friday, 27 July 2012, 10:36
> Subject: Heads-up: new typedef 'svn_merge_rangelist_t'
>
> Heads up: I'm going to add
>
> typedef apr_array_header_t svn_merge_rangelist_t;
>
> There's no major impact; it doesn't change anything functionally.
>
> The rest of the details:
> The immediate reason is to improve the debugging experience: I can then make GDB
> display 'rangelist' values in a simple human-readable form, whereas
> presently the best it can do is recognize that it is an APR array and display
> something like "array of 2 items".
>
> Beyond that, it makes sense from a coding abstraction point of view: it fills in
> the gap in this hierarchy of types (quoting from svn_mergeinfo.h):
>
> /** Terminology for data structures that contain mergeinfo.
> *
> * Subversion commonly uses several data structures to represent
> * mergeinfo in RAM:
> *
> * (a) Strings (@c svn_string_t *) containing "unparsed mergeinfo".
> *
> * (b) A "rangelist". An array (@c apr_array_header_t *) of
> non-overlapping
> * merge ranges (@c svn_merge_range_t *), sorted as said by
> * @c svn_sort_compare_ranges(). An empty range list is [...].
> *
> * (c) @c svn_mergeinfo_t, called "mergeinfo". A hash mapping merge
> * source paths (@c const char *, starting with slashes) to
> * non-empty rangelist arrays. A @c NULL hash is used to [...].
> *
> * (d) @c svn_mergeinfo_catalog_t, called a "mergeinfo catalog". A
> hash
> * mapping paths (@c const char *) to @c svn_mergeinfo_t.
> *
> * Both @c svn_mergeinfo_t and @c svn_mergeinfo_catalog_t are just
> * typedefs for @c apr_hash_t *; there is no static type-checking, and
> * you still use standard @c apr_hash_t functions to interact with
> * them. [...]
> */
>
> typedef apr_hash_t *svn_mergeinfo_t;
> typedef apr_hash_t *svn_mergeinfo_catalog_t;
>
> The new type will be public. Because it's just a typedef, the C API will be
> backward-compatible and it won't change the ABI. (Can someone confirm
> that?)
>
> As mentioned in that doc string, using such a typedef doesn't provide any
> static type-checking.
>
> svn_merge_rangelist_t is not going to be a pointer like svn_mergeinfo_t and
> svn_mergeinfo_catalog_t are, since that is a poor idiom leading to inability to
> declare a parameter as "pointer to a const rangelist".
>
> - Julian
>