Thanks,
Daniele
On 8/6/14, 11:07 AM, "Jarno Rajahalme" <jrajaha...@nicira.com> wrote:
>Daniele,
>
>Thanks for spotting these. GCC error message may be cryptic, but using an
>atomic variable as the return value is an error regardless.
>
>Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
>
>Pushed to master,
>
> Jarno
>
>On Aug 6, 2014, at 10:35 AM, Daniele Di Proietto <ddiproie...@vmware.com>
>wrote:
>
>> There's no reason for the local variable 'old_count' to be atomic. In
>>fact, if
>> it is atomic it triggers a GCC4.9 warning (Wunused-value)
>> The global variables 'a' and 'paux' could be static, according to
>>sparse.
>>
>> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com>
>> ---
>> There's something interesting about the warning: GCC does not complain
>>about
>> 'old_count' being atomic, but about "right-hand operand of comma
>>expression
>> having no effect" (the comma comes from OVS macro
>>atomic_add_explicit()).
>>
>> I've isolated a test case to trigger the warning:
>>
>> /* main.c */
>> #include <stdatomic.h>
>> #include <stdint.h>
>>
>> int main()
>> {
>> _Atomic(uint32_t) a = 0;
>> #ifdef TEST_ATOMIC_DST
>> _Atomic(uint32_t) b = 0;
>> #else
>> uint32_t b = 0;
>> #endif
>> b = atomic_fetch_add_explicit ((&a), (1), (memory_order_seq_cst)),
>> (void) 0;
>>
>> return b;
>> }
>>
>> When compiled with
>>
>> gcc main.c -Wall
>>
>> there's no warning. When compiled with
>>
>> gcc main.c -Wall -DTEST_ATOMIC_DST
>>
>> GCC complains with
>>
>> main.c: In function Œmain¹:
>> main.c:13:70: warning: right-hand operand of comma expression has no
>>effect
>> [-Wunused-value]
>> b = atomic_fetch_add_explicit ((&a), (1), (memory_order_seq_cst)),
>> ^
>> I'm using gcc (Debian 4.9.1-1) 4.9.1
>> ---
>> tests/test-atomic.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/test-atomic.c b/tests/test-atomic.c
>> index ca293ba..31c06fa 100644
>> --- a/tests/test-atomic.c
>> +++ b/tests/test-atomic.c
>> @@ -167,7 +167,7 @@ test_atomic_flag(void)
>> ovs_assert(atomic_flag_test_and_set(&flag) == false);
>> }
>>
>> -uint32_t a;
>> +static uint32_t a;
>>
>> struct atomic_aux {
>> atomic_uint32_t count;
>> @@ -175,7 +175,7 @@ struct atomic_aux {
>> ATOMIC(uint32_t *) data;
>> };
>>
>> -ATOMIC(struct atomic_aux *) paux = ATOMIC_VAR_INIT(NULL);
>> +static ATOMIC(struct atomic_aux *) paux = ATOMIC_VAR_INIT(NULL);
>> static struct atomic_aux *auxes = NULL;
>>
>> #define ATOMIC_ITEM_COUNT 1000000
>> @@ -273,7 +273,7 @@ static void *
>> atomic_writer(void *aux_)
>> {
>> struct atomic_aux *aux = aux_;
>> - atomic_uint32_t old_count;
>> + uint32_t old_count;
>> uint32_t *data;
>> size_t i;
>>
>> --
>> 2.0.1
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>>
>>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman
>>/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=MV9BdLjtFIdhBDBaw5z%2
>>BU6SSA2gAfY4L%2F1HCy3VjlKU%3D%0A&m=XE2zPadAxTGxbIcaeiPkkwVAkmctcz5orBACaG
>>Ux5wU%3D%0A&s=3ffec66cb9cd408188f182595a5da510c43f37589962b80e9fafaa4deb4
>>de90a
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
