Re: [PATCH 2/4] Make _asn1_ordering_* return error values and check them.

[Kurt Roeckx]

On Thu, Apr 24, 2014 at 10:45:24PM +0200, Kurt Roeckx wrote:
> 
> At first sight, it looks like a problem in the
> _asn1_ordering_set_of() function.

So looking at the code it now has:
  p = node->down;
  while (p && ((type_field (p->type) == ASN1_ETYPE_TAG) ||
         (type_field (p->type) == ASN1_ETYPE_SIZE)))
    p = p->right;
  p = p->right;

  if ((p == NULL) || (p->right == NULL))
    return ASN1_VALUE_NOT_FOUND;

My current understanding of things is I think that second
"p = p->right" is wrong.  (The tests actually pass if I
remove it.)  This second one is also not present in the
_asn1_ordering_set() function.  I see no good reason why
we need to move one more entry right.

But I think that the return ASN1_VALUE_NOT_FOUND is
also wrong.  If the set contains 0 or 1 entries there
is nothing to sort and we can just return.

That would give the following diff that works for me:
--- a/lib/coding.c
+++ b/lib/coding.c
@@ -727,7 +729,7 @@ _asn1_ordering_set (unsigned char *der, int der_len, 
asn1_node node)
     p = p->right;
 
   if ((p == NULL) || (p->right == NULL))
-    return ASN1_VALUE_NOT_FOUND;
+    return ASN1_SUCCESS;
 
   first = last = NULL;
   while (p)
@@ -862,10 +864,9 @@ _asn1_ordering_set_of (unsigned char *der, int der_len, 
asn1_node node)
   while (p && ((type_field (p->type) == ASN1_ETYPE_TAG) ||
         (type_field (p->type) == ASN1_ETYPE_SIZE)))
     p = p->right;
-  p = p->right;
 
   if ((p == NULL) || (p->right == NULL))
-    return ASN1_VALUE_NOT_FOUND;
+    return ASN1_SUCCESS;
 
   first = last = NULL;
   while (p)



Kurt