subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1626247 - /subversion/trunk/subversion/libsvn_ra_svn/client.c
Date Mon, 22 Sep 2014 11:42:38 GMT
On 20 September 2014 12:11, Branko ─îibej <brane@wandisco.com> wrote:
> On 19.09.2014 17:21, ivan@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

Mime
View raw message