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 <julianf...@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 <julianf...@btopenworld.com> >> To: Subversion Development <dev@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 >>
