Return-Path: X-Original-To: apmail-subversion-dev-archive@minotaur.apache.org Delivered-To: apmail-subversion-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id D383818E1E for ; Fri, 21 Aug 2015 18:45:19 +0000 (UTC) Received: (qmail 87989 invoked by uid 500); 21 Aug 2015 18:45:19 -0000 Delivered-To: apmail-subversion-dev-archive@subversion.apache.org Received: (qmail 87932 invoked by uid 500); 21 Aug 2015 18:45:19 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 87921 invoked by uid 99); 21 Aug 2015 18:45:19 -0000 Received: from Unknown (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 21 Aug 2015 18:45:19 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id EE12618266B for ; Fri, 21 Aug 2015 18:45:18 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.9 X-Spam-Level: ** X-Spam-Status: No, score=2.9 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=3, RCVD_IN_MSPIKE_H2=-0.001, URIBL_BLOCKED=0.001] autolearn=disabled Authentication-Results: spamd3-us-west.apache.org (amavisd-new); dkim=pass (1024-bit key) header.d=wandisco.com Received: from mx1-us-east.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id 3dulQ7a7KzqT for ; Fri, 21 Aug 2015 18:45:13 +0000 (UTC) Received: from mail-ig0-f171.google.com (mail-ig0-f171.google.com [209.85.213.171]) by mx1-us-east.apache.org (ASF Mail Server at mx1-us-east.apache.org) with ESMTPS id 494D842996 for ; Fri, 21 Aug 2015 18:45:13 +0000 (UTC) Received: by igcse8 with SMTP id se8so6834411igc.1 for ; Fri, 21 Aug 2015 11:45:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wandisco.com; s=gapps; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=0818C0wLiTeBpe3V+iOOigjl6dPJRUVxX31OKObv6Q8=; b=Cu7+qRGUNGU96UGDdY3q+5GuffCQabOOpBqRZVFjwL359xVv6s+M5jR17PIC4+Aetn tCMK5GQHLQjuu8vyOAcy+yPBsbQxmFIzUj1M3q8viu/IWY5HamHq4Nn5z1Lcpl1gPU27 r6GqZ3aGgW0xoCeZPeqjDlSUtstVcOs+/jCW4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=0818C0wLiTeBpe3V+iOOigjl6dPJRUVxX31OKObv6Q8=; b=cwz6OtDEUmC6tTDivQqDLor1Cf1OlnplSQlqhyNzqFzYIajQGTPjnPQZdOspJCw0sy 0HBAyBHLXetsDnsVRCmya+7NdftDuAiuppDkLn1SCQgipJeojFB1vo/uW8QrznZxVru1 YCyGCHAHPtXzpMuSBfiJB3nJ/wkT3p/KRmutYPhe5qJfxoMe6oMEKRsk5ssxEGA9M6Yx SBbPOzubgoNGxnI5sbdcMFmQLr6IhNIu4HduUUAxbwFnCmqAQXdDJTHIHd6t+gJYa/10 l+ak1+CKUCfr38KurR592oWJPL+mi8wAfgfmiUM75Y/WclaMf5i3fsnWPiMo4HH2PnpH 3WMA== X-Gm-Message-State: ALoCoQlUBRcojZTyfusCpdlS+x9tG5/OpTVkjC6ZV2FXMWKItsLTwYm+XcMwu0S+/oQM5KTBrsvJ MIME-Version: 1.0 X-Received: by 10.50.30.196 with SMTP id u4mr4266731igh.11.1440182704569; Fri, 21 Aug 2015 11:45:04 -0700 (PDT) Received: by 10.50.159.132 with HTTP; Fri, 21 Aug 2015 11:45:04 -0700 (PDT) In-Reply-To: <55D4C39A.8060209@wandisco.com> References: <20150819162605.D6837AC006B@hades.apache.org> <55D4C39A.8060209@wandisco.com> Date: Fri, 21 Aug 2015 19:45:04 +0100 Message-ID: Subject: Re: svn commit: r1696626 - /subversion/trunk/subversion/libsvn_ra_serf/util.c From: Stefan Fuhrmann To: =?UTF-8?Q?Branko_=C4=8Cibej?= Cc: Subversion Development Content-Type: multipart/alternative; boundary=047d7bdc11bc84cd4e051dd6aa53 --047d7bdc11bc84cd4e051dd6aa53 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Wed, Aug 19, 2015 at 6:57 PM, Branko =C4=8Cibej wro= te: > On 19.08.2015 18:26, stefan2@apache.org wrote: > > Author: stefan2 > > Date: Wed Aug 19 16:26:05 2015 > > New Revision: 1696626 > > > > URL: http://svn.apache.org/r1696626 > > Log: > > * subversion/libsvn_ra_serf/util.c > > (ssl_convert_serf_failures): When doing array size calculations, refe= r > to > > the array object only. > > > > Modified: > > subversion/trunk/subversion/libsvn_ra_serf/util.c > > > > Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c > > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/u= til.c?rev=3D1696626&r1=3D1696625&r2=3D1696626&view=3Ddiff > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original) > > +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Wed Aug 19 > 16:26:05 2015 > > @@ -65,7 +65,9 @@ ssl_convert_serf_failures(int failures) > > apr_uint32_t svn_failures =3D 0; > > apr_size_t i; > > > > - for (i =3D 0; i < sizeof(serf_failure_map) / (2 * > sizeof(apr_uint32_t)); ++i) > > + for (i =3D 0; > > + i < sizeof(serf_failure_map) / (sizeof(serf_failure_map[0])); > > + ++i) > > { > > if (failures & serf_failure_map[i][0]) > > { > > This one would've been better served by introducing a constant for the > array size ... > I agree and was about to commit the patch. Then I greped the code and found that we use this pattern quite frequently (10s of places). There are also 6 local definitions for an ARRAYLEN or ARRAY_LEN macro that nicely encapsulate the sizeof() magic. Given that people seem to like open arrays (spares the invention of yet another constant), I suggest we introduce SVN_ARRAY_LEN (or SVN__ARRAY_LEN) and use that consistently. -- Stefan^2. --047d7bdc11bc84cd4e051dd6aa53 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On W= ed, Aug 19, 2015 at 6:57 PM, Branko =C4=8Cibej <brane@wandisco.com>= ; wrote:
On 19.08.2015 18:26, stefan2@apache.org wrote:
> Author: stefan2
> Date: Wed Aug 19 16:26:05 2015
> New Revision: 1696626
>
> URL: http://svn.apache.org/r1696626
> Log:
> * subversion/libsvn_ra_serf/util.c
>=C2=A0 =C2=A0(ssl_convert_serf_failures): When doing array size calcula= tions, refer to
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 the array object only.
>
> Modified:
>=C2=A0 =C2=A0 =C2=A0subversion/trunk/subversion/libsvn_ra_serf/util.c >
> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
> URL: http://svn.apache.org/v= iewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=3D1696626&r= 1=3D1696625&r2=3D1696626&view=3Ddiff
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D
> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Wed Aug 19 16:26= :05 2015
> @@ -65,7 +65,9 @@ ssl_convert_serf_failures(int failures)
>=C2=A0 =C2=A0 apr_uint32_t svn_failures =3D 0;
>=C2=A0 =C2=A0 apr_size_t i;
>
> -=C2=A0 for (i =3D 0; i < sizeof(serf_failure_map) / (2 * sizeof(ap= r_uint32_t)); ++i)
> +=C2=A0 for (i =3D 0;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0i < sizeof(serf_failure_map) / (sizeof(= serf_failure_map[0]));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0++i)
>=C2=A0 =C2=A0 =C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (failures & serf_failure_map[i][0])<= br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {

This one would've been better served by introducing a constant for the<= br> array size ...

I agree= and was about to commit the patch. Then I greped
the code and found tha= t we use this pattern quite frequently
= (10s of places). There are also 6 local definitions for an
ARRAYLEN or A= RRAY_LEN macro that nicely encapsulate
the sizeof() magic.

Given = that people seem to like open arrays (spares the
invention of yet another constant), I suggest we introduce
SVN_ARRAY_LEN (or SVN__ARRAY_LEN) and use tha= t
consistently.

-- Stefan^2.
--047d7bdc11bc84cd4e051dd6aa53--