I folded your changes in, and there is no warning/error, when compiling with 'clang' and 'gcc'.
Unit tests all passed, Thanks for your review, do I still need to post a v2 patch? On Mon, Jul 22, 2013 at 12:36 PM, Alex Wang <al...@nicira.com> wrote: > I see, I didn't compile use sparse. > > My gcc is: gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3 > And it didn't issue any error, when sparse is not enabled. > > I should always do that,. > > > On Mon, Jul 22, 2013 at 12:22 PM, Ben Pfaff <b...@nicira.com> wrote: > >> I didn't do anything special to get the warning. With GCC 4.4.5 and >> this patch applied, I see: >> >> ../ofproto/ofproto.c:5037:5: error: incompatible types in conditional >> expression (different base types) >> ../ofproto/ofproto.c:5040:9: error: incompatible types in conditional >> expression (different base types) >> ../ofproto/ofproto.c:5040:9: error: incompatible types in conditional >> expression (different base types) >> ../ofproto/ofproto.c:5037:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:97:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:97:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:107:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:107:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:139:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:139:5: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:219:9: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:219:9: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:258:9: error: incompatible types in conditional >> expression (different base types) >> ../tests/test-heap.c:258:9: error: incompatible types in conditional >> expression (different base types) >> >> On Mon, Jul 22, 2013 at 11:39:41AM -0700, Alex Wang wrote: >> > Hey Ben, >> > >> > Also want to ask how do you get the gcc warning? I compiled with '-O3' >> and >> > didn't see it. >> > >> > Thanks, >> > >> > On Mon, Jul 22, 2013 at 11:35 AM, Alex Wang <al...@nicira.com> wrote: >> > >> > > Thanks Ben, >> > > >> > > I'll recheck that. >> > > >> > > >> > > On Mon, Jul 22, 2013 at 11:30 AM, Ben Pfaff <b...@nicira.com> wrote: >> > > >> > >> On Mon, Jul 22, 2013 at 09:19:57AM -0700, Alex Wang wrote: >> > >> > This commit makes macro function "ASSIGN_CONTAINER()" evaluates >> > >> > to "(void)0". This is to avoid the 'clang' warning: "expression >> > >> > result unused", since most of time, the final evaluated value >> > >> > is not used. >> > >> > >> > >> > Signed-off-by: Alex Wang <al...@nicira.com> >> > >> >> > >> I think you missed a case in heap.h. I had to apply the following to >> > >> avoid a pile of GCC warnings because in "a ? b : c" the b expression >> > >> had type void and the c expression had type int (value 1). Can you >> > >> fold that in and verify that clang accepts it too? >> > >> >> > >> diff --git a/lib/heap.h b/lib/heap.h >> > >> index 9326d79..5c07e04 100644 >> > >> --- a/lib/heap.h >> > >> +++ b/lib/heap.h >> > >> @@ -1,5 +1,5 @@ >> > >> /* >> > >> - * Copyright (c) 2012 Nicira, Inc. >> > >> + * Copyright (c) 2012, 2013 Nicira, Inc. >> > >> * >> > >> * Licensed under the Apache License, Version 2.0 (the "License"); >> > >> * you may not use this file except in compliance with the License. >> > >> @@ -69,13 +69,13 @@ void heap_rebuild(struct heap *); >> > >> #define HEAP_FOR_EACH(NODE, MEMBER, HEAP) >> \ >> > >> for (((HEAP)->n > 0 >> \ >> > >> ? ASSIGN_CONTAINER(NODE, (HEAP)->array[1], MEMBER) >> \ >> > >> - : ((NODE) = NULL, 1)); >> \ >> > >> + : ((NODE) = NULL, (void) 0)); >> \ >> > >> (NODE) != NULL; >> \ >> > >> ((NODE)->MEMBER.idx < (HEAP)->n >> \ >> > >> ? ASSIGN_CONTAINER(NODE, >> \ >> > >> (HEAP)->array[(NODE)->MEMBER.idx + 1], >> \ >> > >> MEMBER) >> \ >> > >> - : ((NODE) = NULL, 1))) >> > >> + : ((NODE) = NULL, (void) 0))) >> > >> >> > >> /* Returns the index of the node that is the parent of the node >> with the >> > >> given >> > >> * 'idx' within a heap. */ >> > >> >> > >> Thanks, >> > >> >> > >> Ben. >> > >> >> > > >> > > >> > >
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
