| Message ID | 4099DE2E54AFAD489356C6C9161D533397458186@dggeml522-mbs.china.huawei.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [ovs-dev] ovs-atomic: __GNUC__ == 4 is enough | expand |
On 9/19/20 8:08 AM, Linhaifeng wrote: > 1. include config.h to avoid include different atomic head file All .c files should include config.h as their first include, so config.h should always be already included at the time we including ovs-atomic.h. Do you know the .c file that doesn't include config.h? If so, please, fix it instead. Header files should not include config.h by themselves. > 2. __GNUC__ == 4 is enough This is technically correct, but it doesn't worth the work for fixing it unless you're fixing some real issue at the same time. Best regards, Ilya Maximets. > > Fixes: 31a3fc6e3e9c ("ovs-atomic: New library for atomic operations.") > Cc: blp@nicira.com > > Signed-off-by: Linhaifeng <haifeng.lin@huawei.com> > --- > lib/ovs-atomic.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h > index 11fa19268..2aee4c608 100644 > --- a/lib/ovs-atomic.h > +++ b/lib/ovs-atomic.h > @@ -311,6 +311,7 @@ > * memory_order_seq_cst for atomic_flag_clear()). > */ > > +#include <config.h> > #include <limits.h> > #include <pthread.h> > #include <stdbool.h> > @@ -331,7 +332,7 @@ > #include "ovs-atomic-c11.h" > #elif __GNUC__ >= 5 && !defined(__cplusplus) > #error "GCC 5+ should have <stdatomic.h>" > - #elif __GNUC__ >= 5 || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 7) > + #elif __GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) > #include "ovs-atomic-gcc4.7+.h" > #elif __GNUC__ && defined(__x86_64__) > #include "ovs-atomic-x86_64.h" >
> > On 9/19/20 8:08 AM, Linhaifeng wrote: > > 1. include config.h to avoid include different atomic head file > > All .c files should include config.h as their first include, so config.h should always > be already included at the time we including ovs-atomic.h. Do you know the .c > file that doesn't include config.h? > If so, please, fix it instead. Header files should not include config.h by > themselves. > OK. Thank you. > > 2. __GNUC__ == 4 is enough > > This is technically correct, but it doesn't worth the work for fixing it unless > you're fixing some real issue at the same time. > OK. Thank you.
diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h index 11fa19268..2aee4c608 100644 --- a/lib/ovs-atomic.h +++ b/lib/ovs-atomic.h @@ -311,6 +311,7 @@ * memory_order_seq_cst for atomic_flag_clear()). */ +#include <config.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -331,7 +332,7 @@ #include "ovs-atomic-c11.h" #elif __GNUC__ >= 5 && !defined(__cplusplus) #error "GCC 5+ should have <stdatomic.h>" - #elif __GNUC__ >= 5 || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 7) + #elif __GNUC__ >= 5 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7) #include "ovs-atomic-gcc4.7+.h" #elif __GNUC__ && defined(__x86_64__) #include "ovs-atomic-x86_64.h"
1. include config.h to avoid include different atomic head file 2. __GNUC__ == 4 is enough Fixes: 31a3fc6e3e9c ("ovs-atomic: New library for atomic operations.") Cc: blp@nicira.com Signed-off-by: Linhaifeng <haifeng.lin@huawei.com> --- lib/ovs-atomic.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)