Re: svn commit: r38381 - in trunk/subversion: include libsvn_subr

[Senthil Kumaran S <se...@collab.net>]

Julian Foad wrote:
> Author: julianfoad
> Date: Thu Jul  9 05:26:08 2009
> New Revision: 38381
...
> +/** Return the key of the hash table entry indexed by @a hi. */
> +const void *svn_apr_hash_index_key(const apr_hash_index_t *hi);

Should "hi" be 'const' here? Since "hi" does not get modified by calling
'apr_hash_this'.

> +/** Return the key length of the hash table entry indexed by @a hi. */
> +apr_ssize_t svn_apr_hash_index_klen(const apr_hash_index_t *hi);

Same here.

> +/** Return the value of the hash table entry indexed by @a hi. */
> +void *svn_apr_hash_index_val(const apr_hash_index_t *hi);

Same here.

Thank You.
-- 
Senthil Kumaran S
http://www.stylesen.org/

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2369633

Re: svn commit: r38381 - in trunk/subversion: include libsvn_subr

[Julian Foad <ju...@btopenworld.com>]

On Fri, 2009-07-10 at 15:47 +0530, Senthil Kumaran S wrote:
> Julian Foad wrote:
> > Author: julianfoad
> > Date: Thu Jul  9 05:26:08 2009
> > New Revision: 38381
> ...
> > +/** Return the key of the hash table entry indexed by @a hi. */
> > +const void *svn_apr_hash_index_key(const apr_hash_index_t *hi);
> 
> Should "hi" be 'const' here? Since "hi" does not get modified by calling
> 'apr_hash_this'.

The argument is a pointer to the hash index structure. Neither the hash
index structure nor the pointer variable 'hi' get modified. I used
"const" on the pointer's target so that the caller can be sure the
function won't modify the pointed-to data.

When Greg said "I'd suggest changing the HI parameter to a const [...]",
I am confident that this is what he meant.

It sounds like you are asking about the argument "hi" itself. A C
function can never change the caller's copy of the argument because
arguments are always passed by value, so there is never any point in
making the argument itself "const".

So this is fine:

void foo(const char *s)
{
    /* skip the first character of the string */
    s++;    /* doesn't affect the caller */
    ...
}

and this is silly:

void foo(const char *const s)
{
    /* within the function body, formal parameter 's' is constant */
}

because it clutters the interface while making no difference to the
caller and having only a minor effect on the implementation.


> > +/** Return the key length of the hash table entry indexed by @a hi. */
> > +apr_ssize_t svn_apr_hash_index_klen(const apr_hash_index_t *hi);
> 
> Same here.
> 
> > +/** Return the value of the hash table entry indexed by @a hi. */
> > +void *svn_apr_hash_index_val(const apr_hash_index_t *hi);
> 
> Same here.
> 
> Thank You.

Thanks for the thought.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2369641