On 20 September 2014 12:11, Branko Čibej <br...@wandisco.com> wrote: > On 19.09.2014 17:21, i...@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
