You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2015/08/19 19:57:46 UTC
Re: svn commit: r1696626 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
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, refer 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/util.c?rev=1696626&r1=1696625&r2=1696626&view=diff
> ==============================================================================
> --- 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 = 0;
> apr_size_t i;
>
> - for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
> + for (i = 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 ...
-- Brane
Re: svn commit: r1696626 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Aug 19, 2015 at 6:57 PM, Branko Čibej <br...@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
> > (ssl_convert_serf_failures): When doing array size calculations, refer
> 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/util.c?rev=1696626&r1=1696625&r2=1696626&view=diff
> >
> ==============================================================================
> > --- 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 = 0;
> > apr_size_t i;
> >
> > - for (i = 0; i < sizeof(serf_failure_map) / (2 *
> sizeof(apr_uint32_t)); ++i)
> > + for (i = 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.