Message ID | 20150905235325.GB23703@nicira.com |
---|---|
State | Not Applicable |
Headers | show |
> On Sep 5, 2015, at 4:53 PM, Ben Pfaff <blp@nicira.com> wrote: > > - printf("%10s %5ld (%s) %s%s\n", acl->direction, acl->priority, > + printf("%10s %5"PRId64" (%s) %s%s\n", acl->direction, acl->priority, Fixed. > I'd prefer to avoid casts in acl_cmp(), also the macro there doesn't > seem to help much: This is better. Thanks. > + if (dir1 != dir2) { > + return dir1 > dir2 ? -1 : 1; I changed this to "<", so that it would sort from smallest to largest, which means "from-lport" comes before "to-lport". > Acked-by: Ben Pfaff <blp@nicira.com> Thanks. Assuming you agree with the above change, I'll keep this ack. --Justin
On Thu, Sep 10, 2015 at 02:07:54PM -0700, Justin Pettit wrote: > > > On Sep 5, 2015, at 4:53 PM, Ben Pfaff <blp@nicira.com> wrote: > > > > - printf("%10s %5ld (%s) %s%s\n", acl->direction, acl->priority, > > + printf("%10s %5"PRId64" (%s) %s%s\n", acl->direction, acl->priority, > > Fixed. > > > I'd prefer to avoid casts in acl_cmp(), also the macro there doesn't > > seem to help much: > > This is better. Thanks. > > > + if (dir1 != dir2) { > > + return dir1 > dir2 ? -1 : 1; > > I changed this to "<", so that it would sort from smallest to largest, which means "from-lport" comes before "to-lport". > > > Acked-by: Ben Pfaff <blp@nicira.com> > > Thanks. Assuming you agree with the above change, I'll keep this ack. Looks good to me, thanks.
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