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

Re: svn commit: r1626247 - /subversion/trunk/subversion/libsvn_ra_svn/client.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Mon, 22 Sep 2014 15:42:38 +0400

On 20 September 2014 12:11, Branko Čibej <brane_at_wandisco.com> wrote:
> On 19.09.2014 17:21, ivan_at_apache.org wrote:
>
> Author: ivan
> Date: Fri Sep 19 15:21:15 2014
> New Revision: 1626247
>
> URL: http://svn.apache.org/r1626247
> Log:
> Resolve another compiler warning.
>
> * subversion/libsvn_ra_svn/client.c
> (find_tunnel_agent): Add non-const local variable ARGV to fix compiler
> warning.
>
>
>
Hi Brane,

> Which compiler is that, and what warning does it emit? There is nothing
> wrong with the code, and I've not seen any warnings from that code in ages,
> with either gcc or clang. Specifically:
>
I was getting warning on client.c:437 (warning C4090: 'function':
different 'const' qualifiers) when compiling using VS2010. The line
437 is memcpy() call:
[[
memcpy(*argv, cmd_argv, n * sizeof(char *));
]]

>
> -/* (Note: *ARGV is an output parameter.) */
> +/* (Note: *ARGV_P is an output parameter.) */
> static svn_error_t *find_tunnel_agent(const char *tunnel,
> const char *hostinfo,
> - const char ***argv,
> + const char ***argv_p,
> apr_hash_t *config, apr_pool_t *pool)
>
>
> The original 'argv' parameter is not constant. It is a non-const pointer to
> a non-const array of (const char*). There should be no warnings when
> assigning to the pointer, and no warnings when modifying the array. The only
> thing you can't do is modify the data the array elements are pointing to,
> but the code did not do that.
>
Thanks for explanation, but I'm also know const pointer syntax.

> Incidentally, I agree with using a for-loop to copy the initial argument
> array instead of a memcpy; it makes the intent of the code clearer.
>
> OTOH, I really do not like code changes that cater to broken compilers.
>
I agree that making code worse just to make broken compilers happy is
not good thing, but I think new code is better and easier to read.

I was planning to fix compiler warning then I realized that code is
actually correct but could be rewritten clearer and fix this
false negative. That what I did in r1626247. But I forget to
update log message that I already wrote (it was part of batch fixes).

---
Ivan Zhakov
Received on 2014-09-22 13:43:26 CEST

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