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

Re: svn commit: r11673 - trunk/subversion/libsvn_subr

From: <kfogel_at_collab.net>
Date: 2004-11-01 15:41:40 CET

lundblad@tigris.org writes:
> --- trunk/subversion/libsvn_subr/utf.c (original)
> +++ trunk/subversion/libsvn_subr/utf.c Sat Oct 30 17:17:58 2004
> @@ -55,6 +55,15 @@
> apr_xlate_t *handle;
> struct xlate_handle_node_t *next;
> } xlate_handle_node_t;
> +
> +/* This maps userdata_key strings to pointers to pointers to the first entry
> + in the linked list of xlate handles.
> + We don't store the pointer to the list head directly in the hash table,
> + since we remove/insert entries at the head in the list in the code below,
> + and we can't use apr_hash_set() in each character translation because that
> + function allocates memory in each call where the value is non-NULL.
> + Since these allocations take place in a global pool, this would be a
> + memory leak. */
> static apr_hash_t *xlate_handle_hash = NULL;

I think it's better to document with actual types, that is, the first
part of this doc string would be something like:

   /* This maps 'char *' userdata_key strings to 'xlate_handle_node_t **'
    * handles to the first entry in the linked list of xlate handles.
    * ...

The exact wording isn't important, just as long as we get the C types

I like the rest of the change, by the way -- the code feels easier to
read this way. One totally nitpicking comment, not about this commit:
in get_xlate_handle_node() we have

   (**ret).next = NULL;

This is the only place where the ".next" syntax is used. Is there
some reason not to do

   (*ret)->next = NULL;


The only reason I ask is that someone might do a search for "->next".
(Note that a similar question applies to the line above it.)


To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 1 17:35:47 2004

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.