Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (athena.apache.org: domain of brane@wandisco.com designates
 74.125.82.175 as permitted sender)
Message-ID: <541D36C5.1030903@wandisco.com>
Date: Sat, 20 Sep 2014 10:11:49 +0200
From: =?UTF-8?B?QnJhbmtvIMSMaWJlag==?= <brane@wandisco.com>
Organization: WANdisco
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9;
 rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: Subversion Development <dev@subversion.apache.org>
Subject: Re: svn commit: r1626247 -
 /subversion/trunk/subversion/libsvn_ra_svn/client.c
References: <20140919152115.6FDAF23889E1@eris.apache.org>
In-Reply-To: <20140919152115.6FDAF23889E1@eris.apache.org>
Content-Type: multipart/alternative;
 boundary="------------000900000200020306060203"

This is a multi-part message in MIME format.
--------------000900000200020306060203
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

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.


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:


> -/* (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.

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.

-- Brane


--------------000900000200020306060203
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 7bit

<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">On 19.09.2014 17:21, <a class="moz-txt-link-abbreviated" href="mailto:ivan@apache.org">ivan@apache.org</a>
      wrote:<br>
    </div>
    <blockquote cite="mid:20140919152115.6FDAF23889E1@eris.apache.org"
      type="cite">
      <pre wrap="">Author: ivan
Date: Fri Sep 19 15:21:15 2014
New Revision: 1626247

URL: <a class="moz-txt-link-freetext" href="http://svn.apache.org/r1626247">http://svn.apache.org/r1626247</a>
Log:
Resolve another compiler warning.

* subversion/libsvn_ra_svn/client.c
  (find_tunnel_agent): Add non-const local variable ARGV to fix compiler 
   warning.</pre>
    </blockquote>
    <br>
    <br>
    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:<br>
    <br>
    <br>
    <blockquote cite="mid:20140919152115.6FDAF23889E1@eris.apache.org"
      type="cite">
      <pre wrap="">-/* (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)</pre>
    </blockquote>
    <br>
    The original 'argv' parameter is <b>not</b> 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.<br>
    <br>
    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.<br>
    <br>
    OTOH, I really do not like code changes that cater to broken
    compilers.<br>
    <br>
    -- Brane<br>
    <br>
  </body>
</html>

--------------000900000200020306060203--