Re: r28302 "Fix warnings ... Cast away const-ness"

Hyrum,

Ping! Not a high priority of course, but I'd still like to get to the bottom of
this.

- Julian

Julian Foad wrote:
>> ------------------------------------------------------------------------
>> r28302 | hwright | 2007-12-06 17:54:23 +0000 (Thu, 06 Dec 2007) | 8 lines
>>
>> Fix compiler warnings on Windows: Cast away const-ness.
>>
>> * subversion/libsvn_diff/diff_file.c,
>> subversion/libsvn_ra_svn/client.c,
>> subversion/libsvn_fs_fs/fs_fs.c:
>> Fix a couple of calls to functions which require pointers to mutable
>> data,
>> but that we have declared const.
>
>
> Hyrum,
>
> Can you explain this for me?
>
>> $ svn diff -c28302
>> Index: subversion/libsvn_diff/diff_file.c
>> ===================================================================
>> --- subversion/libsvn_diff/diff_file.c (revision 28301)
>> +++ subversion/libsvn_diff/diff_file.c (revision 28302)
>> @@ -568,7 +568,7 @@
>> const char **argv = apr_palloc(pool, sizeof(char*) * (args->nelts +
>> 2));
>>
>> argv[0] = "";
>> - memcpy(argv + 1, args->elts, sizeof(char*) * args->nelts);
>> + memcpy((void *) (argv + 1), args->elts, sizeof(char*) * args->nelts);
>> argv[args->nelts + 1] = NULL;
>
>
> I don't get that.
>
> (argv + 1) is same type as argv, i.e. pointer to _mutable_ pointer to
> const-char.
>
> memcpy should expect "void *" i.e. pointer to anything mutable, where
> "anything" includes "pointer to const-char".
>
> This makes me think of (and I've just gone and read up about) a
> limitation in C (at least in C'89) that a type "T **" does not
> automatically convert to "const T **", even though that is a safe and
> desirable conversion. However, I don't see that that is the problem here.
>
> Can you tell what the actual problem was and/or show the actual warning
> message? You see, I'm wondering if it's the compiler behaving badly
> rather than the code being bad.
>
>> Index: subversion/libsvn_ra_svn/client.c
>> ===================================================================
>> --- subversion/libsvn_ra_svn/client.c (revision 28301)
>> +++ subversion/libsvn_ra_svn/client.c (revision 28302)
>> @@ -405,7 +405,7 @@
>> for (n = 0; cmd_argv[n] != NULL; n++)
>> ;
>> *argv = apr_palloc(pool, (n + 4) * sizeof(char *));
>> - memcpy(*argv, cmd_argv, n * sizeof(char *));
>> + memcpy((void *) *argv, cmd_argv, n * sizeof(char *));
>
>
> Same here, but with one more level of indirection.
>
>> (*argv)[n++] = svn_path_uri_decode(hostinfo, pool);
>> (*argv)[n++] = "svnserve";
>> (*argv)[n++] = "-t";
>> Index: subversion/libsvn_fs_fs/fs_fs.c
>> ===================================================================
>> --- subversion/libsvn_fs_fs/fs_fs.c (revision 28301)
>> +++ subversion/libsvn_fs_fs/fs_fs.c (revision 28302)
>> @@ -741,7 +741,7 @@
>> static svn_error_t *
>> purge_shared_txn(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool)
>> {
>> - return with_txnlist_lock(fs, purge_shared_txn_body, &txn_id, pool);
>> + return with_txnlist_lock(fs, purge_shared_txn_body, (char **)
>> &txn_id, pool);
>> }
>
>
> I think I understand this one and have seen other instances of it. It's
> basically that the designer of with_txnlist_lock() wished for that
> parameter to be able to take either a (char **) or a (const char **) -
> as the semantics would be correct with either - but, because of the C
> language limitation mentioned above, the function prototype can only
> permit one or the other, and so some callers have to use a type cast.
>
> - Julian
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>
>

-- 
http://www.foad.me.uk/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org