You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Rainer Jung <ra...@kippdata.de> on 2017/10/25 08:22:46 UTC

Re: svn commit: r1683521 - /apr/apr/trunk/network_io/unix/sockaddr.c

The following comment also applies to the backports to 1.7.x (1808043) 
and 1.6.x (1808043):

Am 04.06.2015 um 13:24 schrieb jorton@apache.org:
> Author: jorton
> Date: Thu Jun  4 11:24:20 2015
> New Revision: 1683521
> 
> URL: http://svn.apache.org/r1683521
> Log:
> * network_io/unix/sockaddr.c (apr_parse_addr_port): Simplify to use
>    apr_pstrmemdup, no functional change.
> 
> Modified:
>      apr/apr/trunk/network_io/unix/sockaddr.c
> 
> Modified: apr/apr/trunk/network_io/unix/sockaddr.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/network_io/unix/sockaddr.c?rev=1683521&r1=1683520&r2=1683521&view=diff
> ==============================================================================
> --- apr/apr/trunk/network_io/unix/sockaddr.c (original)
> +++ apr/apr/trunk/network_io/unix/sockaddr.c Thu Jun  4 11:24:20 2015
> @@ -277,19 +277,13 @@ APR_DECLARE(apr_status_t) apr_parse_addr
>                   return APR_EINVAL;
>               }
>               addrlen = scope_delim - str - 1;
> -            *scope_id = apr_palloc(p, end_bracket - scope_delim);
> -            memcpy(*scope_id, scope_delim + 1, end_bracket - scope_delim - 1);
> -            (*scope_id)[end_bracket - scope_delim - 1] = '\0';
> +            *scope_id = apr_pstrmemdup(p, scope_delim, end_bracket - scope_delim - 1);

Before the change, copying starts at "scope_delim + 1", after the change 
at "scope_delim". The log says "Simplify to use apr_pstrmemdup, no 
functional change.", so it seems that was not an intentional change. 
Shouldn't we correct it with the following change:

Index: network_io/unix/sockaddr.c
===================================================================
--- network_io/unix/sockaddr.c  (revision 1809650)
+++ network_io/unix/sockaddr.c  (working copy)
@@ -277,7 +277,7 @@
                  return APR_EINVAL;
              }
              addrlen = scope_delim - str - 1;
-            *scope_id = apr_pstrmemdup(p, scope_delim, end_bracket - 
scope_delim - 1);
+            *scope_id = apr_pstrmemdup(p, scope_delim + 1, end_bracket 
- scope_delim - 1);
          }
          else {
              addrlen = addrlen - 2; /* minus 2 for '[' and ']' */

Regards,

Rainer

Re: svn commit: r1683521 - /apr/apr/trunk/network_io/unix/sockaddr.c

Posted by Rainer Jung <ra...@kippdata.de>.
Am 25.10.2017 um 15:21 schrieb Joe Orton:
> On Wed, Oct 25, 2017 at 10:22:46AM +0200, Rainer Jung wrote:
>> Before the change, copying starts at "scope_delim + 1", after the change at
>> "scope_delim". The log says "Simplify to use apr_pstrmemdup, no functional
>> change.", so it seems that was not an intentional change. Shouldn't we
>> correct it with the following change:
> 
> How annoying, sorry :( Yes you're obviously right.

NP, thanks a bunch for your prompt reaction!

Rainer



Re: svn commit: r1683521 - /apr/apr/trunk/network_io/unix/sockaddr.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Oct 25, 2017 at 10:22:46AM +0200, Rainer Jung wrote:
> Before the change, copying starts at "scope_delim + 1", after the change at
> "scope_delim". The log says "Simplify to use apr_pstrmemdup, no functional
> change.", so it seems that was not an intentional change. Shouldn't we
> correct it with the following change:

How annoying, sorry :( Yes you're obviously right.

http://svn.apache.org/viewvc?view=revision&revision=1813286