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

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

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2007-12-15 01:50:14 CET

> ------------------------------------------------------------------------
> 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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Dec 15 02:03:53 2007

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