[ovs-dev] [PATCH 2/3] clang: Fix the "expression result unused" warning.

[Alex Wang]

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 <alexw at 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 <blp at 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 <alexw at nicira.com> wrote:
>> >
>> > > Thanks Ben,
>> > >
>> > > I'll recheck that.
>> > >
>> > >
>> > > On Mon, Jul 22, 2013 at 11:30 AM, Ben Pfaff <blp at 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 <alexw at 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.
>> > >>
>> > >
>> > >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130722/14d11e7d/attachment-0003.html>