kfogel@collab.net wrote:
> Julian Foad <julianfoad@btopenworld.com> writes:
>
>>Stop defining Subversion identifiers in APR's name space.
>> (apr_hash_sorted_keys): Rename to "svn_sort_hash".
>> (apr_atoui64): Rename this macro to "svn_atoui64".
In this revised patch I have used "svn_sort__hash" and "svn__atoui64".
> But also, Greg Stein made an interesting suggestion, quoted in the
> issue (#1644):
>
> > If we're pretty sure these will get into APR, we could name them
> > svn__apr_foo and SVN__APR_FOO, make sure there are no doxygen
> > comments for them, and note in the header files that they're for
> > internal use only. Then it should be reasonably safe to remove
> > them from the libraries in the future.
> >
> > If we think some of them won't make it into APR, then we should
> > pick names we're comfortable with supporting.
> I'm not sure, right now, which of these are likely to make it into
> APR, but its seems prudent to start out by using auto-deprecated
> names. We can always promote them to real names in a later release
> once we know what's happened in APR.
>
> To that end, the doc strings for these functions should also say that
> these names should not be depended on, and should probably point to
> the issue for further detail.
I have noted in the header files that they are private and referred to issue #1644. I have not removed the Doxygen formatting as I'm not sure whether that's for the best; please do so if you're sure.
> Regarding svn_sort_hash(), do we want to also rename svn_item_t to
> match?
Renamed to "svn_sort__item_t" in this patch. (Have not renamed "svn_sort_compare_items_as_paths" but it is implicitly private because it uses a private type for its arguments. Sure, explicit would be better, but another time.)
Please feel free to modify and/or commit this if you don't hear from me in the next day or so.
- Julian
Stop defining Subversion identifiers in APR's name space (issue #1644).
* subversion/include/svn_sorts.h
* subversion/libsvn_subr/sorts.c
(svn_sort__item_t): New name for "svn_item_t".
(svn_sort__hash): New name for "apr_hash_sorted_keys".
(apr_array_prepend): Delete, as it was unused.
* subversion/include/svn_types.h
(svn__atoui64): New name for "apr_atoui64".
* subversion/bindings/java/javahl/native/SVNClient.cpp (list):
* subversion/clients/cmdline/log-cmd.c (log_message_receiver):
* subversion/clients/cmdline/ls-cmd.c (print_dirents):
* subversion/libsvn_client/blame.c (log_message_receiver):
* subversion/libsvn_fs/util/fs_skels.c (svn_fs__parse_representation_skel):
* subversion/libsvn_ra_dav/fetch.c (svn_ra_dav__get_dir):
* subversion/libsvn_repos/load.c (svn_repos_parse_dumpstream):
* subversion/mod_dav_svn/repos.c (dav_svn_deliver):
Adjust references accordingly.
Index: subversion/include/svn_sorts.h
===================================================================
--- subversion/include/svn_sorts.h (revision 8049)
+++ subversion/include/svn_sorts.h (working copy)
@@ -34,7 +34,9 @@
-/** This structure is used to hold a key/value from a hash table */
+/** This structure is used to hold a key/value from a hash table.
+ * NOTE: Private. For use by Subversion's own code only. See issue #1644.
+ */
typedef struct {
/** pointer to the key */
const void *key;
@@ -44,10 +46,10 @@
/** pointer to the value */
void *value;
-} svn_item_t;
+} svn_sort__item_t;
-/** Compare two @c svn_item_t's, returning an integer greater than,
+/** Compare two @c svn_sort__item_t's, returning an integer greater than,
* equal to, or less than 0, according as @a a is greater than, equal to,
* or less than @a b.
*
@@ -56,13 +58,13 @@
* array, do this:
*
*<pre> apr_array_header_t *hdr;
- * hdr = apr_hash_sorted_keys (hsh, @c svn_sort_compare_items_as_paths,
- * pool);</pre>
+ * hdr = svn_sort__hash (hsh, @c svn_sort_compare_items_as_paths, pool);</pre>
*
* The key strings must be null-terminated, even though klen does not
* include the terminator.
*/
-int svn_sort_compare_items_as_paths (const svn_item_t *a, const svn_item_t *b);
+int svn_sort_compare_items_as_paths (const svn_sort__item_t *a,
+ const svn_sort__item_t *b);
/** Compare two @c svn_revnum_t's, returning an integer greater than, equal
@@ -77,42 +79,28 @@
int svn_sort_compare_revisions (const void *a, const void *b);
-#ifndef apr_hash_sorted_keys
/** Sort @a ht according to its keys, return an @c apr_array_header_t
- * containing @c svn_item_t structures holding those keys and values
- * (i.e. for each @c svn_item_t @a item in the returned array, @a item->key
- * and @a item->size are the hash key, and @a item->data points to
+ * containing @c svn_sort__item_t structures holding those keys and values
+ * (i.e. for each @c svn_sort__item_t @a item in the returned array,
+ * @a item->key and @a item->size are the hash key, and @a item->data points to
* the hash value).
*
* Storage is shared with the original hash, not copied.
*
- * @a comparison_func should take two @c svn_item_t's and return an integer
- * greater than, equal to, or less than 0, according as the first item
+ * @a comparison_func should take two @c svn_sort__item_t's and return an
+ * integer greater than, equal to, or less than 0, according as the first item
* is greater than, equal to, or less than the second.
*
- * NOTE:
- * This function and the @a svn_item_t should go over to APR. Got a Round Tuit?
- */
-apr_array_header_t *
-apr_hash_sorted_keys (apr_hash_t *ht,
- int (*comparison_func) (const svn_item_t *,
- const svn_item_t *),
- apr_pool_t *pool);
-#endif /* apr_hash_sorted_keys */
-
-#ifndef apr_array_prepend
-/** Return a pointer to an empty slot at the head of array @a arr.
- *
- * Note that if your application does not strictly need the new
- * element to be at the head of the array, consider using the much
- * more efficient function, apr_array_push() (which will add your new
- * element at the tail of the array).
+ * NOTE: Private. For use by Subversion's own code only. See issue #1644.
*
- * NOTE:
- * This function should go over to APR.
+ * NOTE: This function and the @a svn_sort__item_t should go over to APR.
*/
-void *apr_array_prepend (apr_array_header_t *arr);
-#endif /* apr_array_prepend */
+apr_array_header_t *
+svn_sort__hash (apr_hash_t *ht,
+ int (*comparison_func) (const svn_sort__item_t *,
+ const svn_sort__item_t *),
+ apr_pool_t *pool);
+
#ifdef __cplusplus
}
Index: subversion/include/svn_ra_svn.h
===================================================================
--- subversion/include/svn_ra_svn.h (revision 8049)
+++ subversion/include/svn_ra_svn.h (working copy)
@@ -210,7 +210,7 @@
svn_error_t *svn_ra_svn_skip_leading_garbage(svn_ra_svn_conn_t *conn,
apr_pool_t *pool);
-/** Parse an array of @c svn_item_t structures as a tuple, using a
+/** Parse an array of @c svn_sort__item_t structures as a tuple, using a
* printf-like interface. The format string @a fmt may contain:
*
*<pre>
Index: subversion/include/svn_types.h
===================================================================
--- subversion/include/svn_types.h (revision 8049)
+++ subversion/include/svn_types.h (working copy)
@@ -122,8 +122,10 @@
/** In @c printf()-style functions, format file sizes using this. */
#define SVN_FILESIZE_T_FMT APR_UINT64_T_FMT
-/* FIXME: Have to fiddle with APR to define this function */
-#define apr_atoui64(X) ((apr_uint64_t) apr_atoi64(X))
+/* Parse a base-10 numeric string into a 64-bit unsigned numeric value. */
+/* NOTE: Private. For use by Subversion's own code only. See issue #1644. */
+/* FIXME: APR should supply a function to do this, such as "apr_atoui64". */
+#define svn__atoui64(X) ((apr_uint64_t) apr_atoi64(X))
/** YABT: Yet Another Boolean Type */
Index: subversion/libsvn_fs/util/fs_skels.c
===================================================================
--- subversion/libsvn_fs/util/fs_skels.c (revision 8049)
+++ subversion/libsvn_fs/util/fs_skels.c (working copy)
@@ -555,7 +555,7 @@
window_skel->children->next->next->data,
window_skel->children->next->next->len);
chunk->offset =
- apr_atoui64 (apr_pstrmemdup (pool,
+ svn__atoui64 (apr_pstrmemdup (pool,
chunk_skel->children->data,
chunk_skel->children->len));
Index: subversion/libsvn_subr/sorts.c
===================================================================
--- subversion/libsvn_subr/sorts.c (revision 8049)
+++ subversion/libsvn_subr/sorts.c (working copy)
@@ -31,7 +31,7 @@
-/*** apr_hash_sorted_keys() ***/
+/*** svn_sort__hash() ***/
/* (Should this be a permanent part of APR?)
@@ -51,7 +51,7 @@
Therefore, it makes sense to store pointers to {void *, size_t}
structures in our array. No such apr object exists... BUT... if we
- can use a new type svn_item_t which contains {char *, size_t, void
+ can use a new type svn_sort__item_t which contains {char *, size_t, void
*}. If store these objects in our array, we get the hash value
*for free*. When looping over the final array, we don't need to
call apr_hash_get(). Major bonus!
@@ -59,7 +59,8 @@
int
-svn_sort_compare_items_as_paths (const svn_item_t *a, const svn_item_t *b)
+svn_sort_compare_items_as_paths (const svn_sort__item_t *a,
+ const svn_sort__item_t *b)
{
const char *astr, *bstr;
@@ -84,25 +85,22 @@
}
-#ifndef apr_hash_sort_keys
-
-/* see svn_sorts.h for documentation */
apr_array_header_t *
-apr_hash_sorted_keys (apr_hash_t *ht,
- int (*comparison_func) (const svn_item_t *,
- const svn_item_t *),
- apr_pool_t *pool)
+svn_sort__hash (apr_hash_t *ht,
+ int (*comparison_func) (const svn_sort__item_t *,
+ const svn_sort__item_t *),
+ apr_pool_t *pool)
{
apr_hash_index_t *hi;
apr_array_header_t *ary;
/* allocate an array with only one element to begin with. */
- ary = apr_array_make (pool, 1, sizeof(svn_item_t));
+ ary = apr_array_make (pool, 1, sizeof(svn_sort__item_t));
/* loop over hash table and push all keys into the array */
for (hi = apr_hash_first (pool, ht); hi; hi = apr_hash_next (hi))
{
- svn_item_t *item = apr_array_push (ary);
+ svn_sort__item_t *item = apr_array_push (ary);
apr_hash_this (hi, &item->key, &item->klen, &item->value);
}
@@ -113,7 +111,6 @@
return ary;
}
-#endif /* apr_hash_sort_keys */
@@ -297,21 +294,3 @@
return svn_prop_is_svn_prop (propname);
}
-
-
-void *
-apr_array_prepend (apr_array_header_t *arr)
-{
- /* Call apr_array_push() to ensure that enough room has been
- alloced. */
- apr_array_push (arr);
-
- /* Now, shift all the things in the array down one spot. */
- memmove (arr->elts + arr->elt_size,
- arr->elts,
- ((arr->nelts - 1) * arr->elt_size));
-
- /* Finally, return the pointer to the first array member so our
- caller could put stuff there. */
- return arr->elts;
-}
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 8049)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -273,14 +273,15 @@
apr_array_header_t *paths;
/* Build a sorted list of the changed paths. */
- paths = apr_hash_sorted_keys (changed_paths,
- svn_sort_compare_items_as_paths, pool);
+ paths = svn_sort__hash (changed_paths,
+ svn_sort_compare_items_as_paths, pool);
/* Now, walk the list of paths backwards, looking a parent of
our path that has copyfrom information. */
for (i = paths->nelts; i > 0; i--)
{
- svn_item_t item = APR_ARRAY_IDX (paths, i - 1, svn_item_t);
+ svn_sort__item_t item = APR_ARRAY_IDX (paths, i - 1,
+ svn_sort__item_t);
const char *ch_path = item.key;
int len = strlen (ch_path);
Index: subversion/bindings/java/javahl/native/SVNClient.cpp
===================================================================
--- subversion/bindings/java/javahl/native/SVNClient.cpp (revision 8049)
+++ subversion/bindings/java/javahl/native/SVNClient.cpp (working copy)
@@ -146,7 +146,7 @@
if (Err == NULL)
{
apr_array_header_t *array =
- apr_hash_sorted_keys (dirents, svn_sort_compare_items_as_paths,
+ svn_sort__hash (dirents, svn_sort_compare_items_as_paths,
subPool.pool());
// create the array of DirEntry
@@ -169,10 +169,10 @@
for (int i = 0; i < array->nelts; i++)
{
- const svn_item_t *item;
+ const svn_sort__item_t *item;
svn_dirent_t *dirent = NULL;
- item = &APR_ARRAY_IDX (array, i, const svn_item_t);
+ item = &APR_ARRAY_IDX (array, i, const svn_sort__item_t);
dirent = (svn_dirent_t *) item->value;
jobject obj = createJavaDirEntry((const char *)item->key, dirent);
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c (revision 8049)
+++ subversion/mod_dav_svn/repos.c (working copy)
@@ -2001,14 +2001,15 @@
}
/* get a sorted list of the entries */
- sorted = apr_hash_sorted_keys(entries, svn_sort_compare_items_as_paths,
- resource->pool);
+ sorted = svn_sort__hash(entries, svn_sort_compare_items_as_paths,
+ resource->pool);
entry_pool = svn_pool_create(resource->pool);
for (i = 0; i < sorted->nelts; ++i)
{
- const svn_item_t *item = &APR_ARRAY_IDX(sorted, i, const svn_item_t);
+ const svn_sort__item_t *item = &APR_ARRAY_IDX(sorted, i,
+ const svn_sort__item_t);
const svn_fs_dirent_t *entry = item->value;
const char *name = item->key;
const char *href = name;
Index: subversion/clients/cmdline/ls-cmd.c
===================================================================
--- subversion/clients/cmdline/ls-cmd.c (revision 8049)
+++ subversion/clients/cmdline/ls-cmd.c (working copy)
@@ -36,7 +36,7 @@
/*** Code. ***/
static int
-compare_items_as_paths (const svn_item_t *a, const svn_item_t *b)
+compare_items_as_paths (const svn_sort__item_t *a, const svn_sort__item_t *b)
{
return svn_path_compare_paths ((const char *)a->key, (const char *)b->key);
}
@@ -49,15 +49,15 @@
apr_array_header_t *array;
int i;
- array = apr_hash_sorted_keys (dirents, compare_items_as_paths, pool);
+ array = svn_sort__hash (dirents, compare_items_as_paths, pool);
for (i = 0; i < array->nelts; ++i)
{
const char *utf8_entryname, *stdout_entryname;
svn_dirent_t *dirent;
- svn_item_t *item;
+ svn_sort__item_t *item;
- item = &APR_ARRAY_IDX (array, i, svn_item_t);
+ item = &APR_ARRAY_IDX (array, i, svn_sort__item_t);
utf8_entryname = item->key;
Index: subversion/clients/cmdline/log-cmd.c
===================================================================
--- subversion/clients/cmdline/log-cmd.c (revision 8049)
+++ subversion/clients/cmdline/log-cmd.c (working copy)
@@ -262,15 +262,15 @@
int i;
/* Get an array of sorted hash keys. */
- sorted_paths = apr_hash_sorted_keys (changed_paths,
- svn_sort_compare_items_as_paths,
- pool);
+ sorted_paths = svn_sort__hash (changed_paths,
+ svn_sort_compare_items_as_paths, pool);
SVN_ERR (svn_stream_printf (lb->out, pool,
"Changed paths:" APR_EOL_STR));
for (i = 0; i < sorted_paths->nelts; i++)
{
- svn_item_t *item = &(APR_ARRAY_IDX (sorted_paths, i, svn_item_t));
+ svn_sort__item_t *item = &(APR_ARRAY_IDX (sorted_paths, i,
+ svn_sort__item_t));
const char *path_stdout, *path = item->key;
svn_log_changed_path_t *log_item
= apr_hash_get (changed_paths, item->key, item->klen);
Index: subversion/libsvn_repos/load.c
===================================================================
--- subversion/libsvn_repos/load.c (revision 8049)
+++ subversion/libsvn_repos/load.c (working copy)
@@ -508,7 +508,7 @@
SVN_ERR (parse_fns->remove_node_props (node_baton));
SVN_ERR (parse_property_block (stream,
- apr_atoui64 (valstr),
+ svn__atoui64 (valstr),
parse_fns,
found_node ? node_baton : rev_baton,
found_node,
@@ -521,7 +521,7 @@
APR_HASH_KEY_STRING)))
{
SVN_ERR (parse_text_block (stream,
- apr_atoui64 (valstr),
+ svn__atoui64 (valstr),
parse_fns,
found_node ? node_baton : rev_baton,
buffer,
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 8049)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -928,7 +928,7 @@
if (propval == NULL)
entry->size = 0;
else
- entry->size = apr_atoui64(propval->data);
+ entry->size = svn__atoui64(propval->data);
/* does this resource contain any 'svn' or 'custom' properties,
i.e. ones actually created and set by the user? */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 20 01:34:04 2003