Re: [ovs-dev] [PATCH 2/2] ovn-nbctl: Add ACL commands.

[Ben Pfaff]

On Fri, Sep 04, 2015 at 05:39:02PM -0700, Justin Pettit wrote:
> Signed-off-by: Justin Pettit <jpet...@nicira.com>

I came really close to duplicating your work here on Thursday when I was
writing tests.  I'm glad that didn't happen.

The printf() specifier is wrong here:

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 0b19521..26a4735 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -824,7 +824,7 @@ do_acl_list(struct ovs_cmdl_context *ctx)
 
     for (i = 0; i < lswitch->n_acls; i++) {
         const struct nbrec_acl *acl = acls[i];
-        printf("%10s %5ld (%s) %s%s\n", acl->direction, acl->priority,
+        printf("%10s %5"PRId64" (%s) %s%s\n", acl->direction, acl->priority,
                 acl->match, acl->action, acl->log ? " log" : "");
     }
 
I'd prefer to avoid casts in acl_cmp(), also the macro there doesn't
seem to help much:

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 0b19521..384bc7b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -775,31 +775,20 @@ dir_encode(const char *dir)
 static int
 acl_cmp(const void *acl1_, const void *acl2_)
 {
-    const struct nbrec_acl *acl1, *acl2;
-
-    acl1 = *((struct nbrec_acl **) acl1_);
-    acl2 = *((struct nbrec_acl **) acl2_);
+    const struct nbrec_acl *const *acl1p = acl1_;
+    const struct nbrec_acl *const *acl2p = acl2_;
+    const struct nbrec_acl *acl1 = *acl1p;
+    const struct nbrec_acl *acl2 = *acl2p;
 
     int dir1 = dir_encode(acl1->direction);
     int dir2 = dir_encode(acl2->direction);
-
-#define CMP(expr) \
-    do { \
-        int res; \
-        res = (expr); \
-        if (res) { \
-            return res; \
-        } \
-    } while (0)
-
-    CMP(dir1 - dir2);
-    CMP(acl1->priority > acl2->priority ? -1 :
-            (acl1->priority < acl2->priority ? 1 : 0));
-    CMP(strcmp(acl1->match, acl2->match));
-
-#undef CMP
-
-    return 0;
+    if (dir1 != dir2) {
+        return dir1 > dir2 ? -1 : 1;
+    } else if (acl1->priority != acl2->priority) {
+        return acl1->priority > acl2->priority ? -1 : 1;
+    } else {
+        return strcmp(acl1->match, acl2->match);
+    }
 }
 
 static void

Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev