Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Subject: Re: svn commit: r1694533 -
 /subversion/trunk/subversion/libsvn_ra_svn/marshal.c
To: dev@subversion.apache.org
References: <20150806161040.14AE5AC01A7@hades.apache.org>
From: =?UTF-8?Q?Branko_=c4=8cibej?= <brane@wandisco.com>
Organization: WANdisco
Message-ID: <55DC5B26.50002@wandisco.com>
Date: Tue, 25 Aug 2015 14:10:14 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:38.0)
 Gecko/20100101 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <20150806161040.14AE5AC01A7@hades.apache.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

On 06.08.2015 18:10, stefan2@apache.org wrote:
> Author: stefan2
> Date: Thu Aug  6 16:10:39 2015
> New Revision: 1694533
>
> URL: http://svn.apache.org/r1694533
> Log:
> Fix an alignment problem on machines with 32 bit pointers but atomic 64
> data access that may not be misaligned.
>
> * subversion/libsvn_ra_svn/marshal.c
>   (read_item): Ensure that the array contents are always have the APR
>                default alignment.
>
> Found by: Rainer Jung <rainer.jung{_AT_}kippdata.de>
>
> Modified:
>     subversion/trunk/subversion/libsvn_ra_svn/marshal.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_svn/marshal.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/marshal.c?rev=1694533&r1=1694532&r2=1694533&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_svn/marshal.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/marshal.c Thu Aug  6 16:10:39 2015
> @@ -1190,14 +1190,20 @@ static svn_error_t *read_item(svn_ra_svn
>      }
>    else if (c == '(')
>      {
> +      /* On machines with 32 bit pointers, array headers are only 20 bytes
> +       * which is not enough for our standard 64 bit alignment.
> +       * So, determine a suitable block size for the APR array header that
> +       * keeps proper alignment for following structs. */
> +      const apr_size_t header_size
> +        = APR_ALIGN_DEFAULT(sizeof(apr_array_header_t));
> +
>        /* Allocate an APR array with room for (initially) 4 items.
>         * We do this manually because lists are the most frequent protocol
>         * element, often used to frame a single, optional value.  We save
>         * about 20% of total protocol handling time. */
> -      char *buffer = apr_palloc(pool, sizeof(apr_array_header_t)
> -                                      + 4 * sizeof(svn_ra_svn_item_t));
> -      svn_ra_svn_item_t *data
> -        = (svn_ra_svn_item_t *)(buffer + sizeof(apr_array_header_t));
> +      char *buffer = apr_palloc(pool,
> +                                header_size + 4 * sizeof(svn_ra_svn_item_t));
> +      svn_ra_svn_item_t *data = (svn_ra_svn_item_t *)(buffer + header_size);
>  
>        item->kind = SVN_RA_SVN_LIST;
>        item->u.list = (apr_array_header_t *)buffer;


How exactly is all this different from:

        apr_array_make(pool, 4, sizeof(svn_ra_svn_item_t)?

Other than relying on magic knowledge of APR implementation details?

-- Brane