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