Re: [PATCH] Use the smallest of the 2 lengths for the comparison

[Kurt Roeckx]

On Mon, Apr 28, 2014 at 04:17:10PM +0200, Nikos Mavrogiannopoulos wrote:
> On Sun, Apr 27, 2014 at 11:17 PM, Kurt Roeckx <address@hidden> wrote:
> > ---
> >  lib/coding.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > diff --git a/lib/coding.c b/lib/coding.c
> > index 15c87d3..34a204b 100644
> > --- a/lib/coding.c
> > +++ b/lib/coding.c
> > @@ -925,7 +925,7 @@ _asn1_ordering_set_of (unsigned char *der, int der_len, 
> > asn1_node node)
> >        counter = 0;
> >        while (p2_vet)
> >         {
> > -         if ((p_vet->end - counter) > (p2_vet->end - p_vet->end))
> > +         if ((p_vet->end - counter) < (p2_vet->end - p_vet->end))
> >             max = p_vet->end - counter;
> >           else
> >             max = p2_vet->end - p_vet->end;
> 
> Hello,
>  I've added the minmax gnulib module so that code like that could be
> expressed as max = MAX(x, y) (or MIN). However, I think that storing
> the minimum value in a variable called max is quite confusing. Is your
> change for safety reasons or mandated by DER?

It's really mostly about DER.

You're basicly doing 2 string compares and you'll stop comparing
after you see a difference or after "max" bytes because that would
be the smallest of the 2 strings.  If you continue after that
you're going to compare things you shouldn't compare anymore and
possible read past the end of the array.

You can argue that "max" is a good name of the variable or not.
At the point you put a value in it it's the minimum of 2
sizes, but then it'll be the max size you should compare.


Kurt