'#else' is useless, need remove.
Signed-off-by: Chen Gang <[email protected]>
---
include/linux/coda.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/include/linux/coda.h b/include/linux/coda.h
index cff544f..d30209b 100644
--- a/include/linux/coda.h
+++ b/include/linux/coda.h
@@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
#if defined(__linux__)
typedef unsigned long long u_quad_t;
-#else
#endif
#include <uapi/linux/coda.h>
#endif
--
1.7.7.6
On Tue, 30 Jul 2013, Chen Gang wrote:
> '#else' is useless, need remove.
>
> Signed-off-by: Chen Gang <[email protected]>
> ---
> include/linux/coda.h | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/coda.h b/include/linux/coda.h
> index cff544f..d30209b 100644
> --- a/include/linux/coda.h
> +++ b/include/linux/coda.h
> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>
> #if defined(__linux__)
> typedef unsigned long long u_quad_t;
> -#else
> #endif
> #include <uapi/linux/coda.h>
> #endif
Applied.
--
Jiri Kosina
SUSE Labs
On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
> '#else' is useless, need remove.
>
> Signed-off-by: Chen Gang <[email protected]>
> ---
> include/linux/coda.h | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/coda.h b/include/linux/coda.h
> index cff544f..d30209b 100644
> --- a/include/linux/coda.h
> +++ b/include/linux/coda.h
> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>
> #if defined(__linux__)
> typedef unsigned long long u_quad_t;
> -#else
> #endif
> #include <uapi/linux/coda.h>
> #endif
Why have the #if at all?
On 07/30/2013 06:38 PM, Jiri Kosina wrote:
> On Tue, 30 Jul 2013, Chen Gang wrote:
>
>> '#else' is useless, need remove.
>>
>> Signed-off-by: Chen Gang <[email protected]>
>> ---
>> include/linux/coda.h | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>> index cff544f..d30209b 100644
>> --- a/include/linux/coda.h
>> +++ b/include/linux/coda.h
>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>
>> #if defined(__linux__)
>> typedef unsigned long long u_quad_t;
>> -#else
>> #endif
>> #include <uapi/linux/coda.h>
>> #endif
>
> Applied.
>
Thanks.
--
Chen Gang
On 07/30/2013 08:29 PM, Joe Perches wrote:
> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>> '#else' is useless, need remove.
>>
>> Signed-off-by: Chen Gang <[email protected]>
>> ---
>> include/linux/coda.h | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>> index cff544f..d30209b 100644
>> --- a/include/linux/coda.h
>> +++ b/include/linux/coda.h
>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>
>> #if defined(__linux__)
>> typedef unsigned long long u_quad_t;
>> -#else
>> #endif
>> #include <uapi/linux/coda.h>
>> #endif
>
> Why have the #if at all?
>
>
>
Hmm... some old version compilers do not define __linux__ automatically
(e.g. or32-linux-gcc 4.5.1-or32-1.0rc1, which is the latest cross
compiler for openrisc, though).
If not define __linux__, the compiler will report error (u_quad_t is
not defined).
When we remove "#if defined(__linux__)" from "include/linux/coda.h",
the compiler will not report error, and 'cdev_t' will be defined as
'dev_t', not 'u_quad_t' (I guess, it is an implicit bug).
In "uapi/include/*", only "coda.h" has "#if defined(__linux__)", maybe
they want multiple OS can share the same "coda.h" file (at least, we
can not say it is a bad idea).
Neither suitable to define __linux__ in "include/linux/coda.h".
All together, I think:
the direct cause:
"uapi/include/coda.h" wants to share itself for multiple OS (so they need check __linux__).
(in "uapi/include/*", only coda.h need check __linux__)
the root cause:
the compiler for linux should define __linux__ automatically, the latest version of gcc has done, but some of the old version is not.
most of cross compilers have merged their code into gcc main tree, but still left some (e.g. openrisc).
(at least, I can not build openrisc from gcc main tree correctly, but most of others can)
some of latest cross compilers still use old version of gcc, (still not define __linux__ automatically).
The related code in "uapi/linux/coda.h" is below:
-------------------------------code begin-------------------------------
73 #if defined(DJGPP) || defined(__CYGWIN32__)
74 #ifdef KERNEL
75 typedef unsigned long u_long;
76 typedef unsigned int u_int;
77 typedef unsigned short u_short;
78 typedef u_long ino_t;
79 typedef u_long dev_t;
80 typedef void * caddr_t;
81 #ifdef DOS
82 typedef unsigned __int64 u_quad_t;
83 #else
84 typedef unsigned long long u_quad_t;
85 #endif
86
87 #define inline
88
89 struct timespec {
90 long ts_sec;
91 long ts_nsec;
92 };
93 #else /* DJGPP but not KERNEL */
94 #include <sys/time.h>
95 typedef unsigned long long u_quad_t;
96 #endif /* !KERNEL */
97 #endif /* !DJGPP */
98
99
100 #if defined(__linux__)
101 #include <linux/time.h>
102 #define cdev_t u_quad_t
103 #ifndef __KERNEL__
104 #if !defined(_UQUAD_T_) && (!defined(__GLIBC__) || __GLIBC__ < 2)
105 #define _UQUAD_T_ 1
106 typedef unsigned long long u_quad_t;
107 #endif
108 #endif /* __KERNEL__ */
109 #else
110 #define cdev_t dev_t
111 #endif
112
-------------------------------code end---------------------------------
Thanks.
--
Chen Gang
On 07/31/2013 09:44 AM, Chen Gang wrote:
> On 07/30/2013 08:29 PM, Joe Perches wrote:
>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>> '#else' is useless, need remove.
>>>
>>> Signed-off-by: Chen Gang <[email protected]>
>>> ---
>>> include/linux/coda.h | 1 -
>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>> index cff544f..d30209b 100644
>>> --- a/include/linux/coda.h
>>> +++ b/include/linux/coda.h
>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>
>>> #if defined(__linux__)
>>> typedef unsigned long long u_quad_t;
>>> -#else
>>> #endif
>>> #include <uapi/linux/coda.h>
>>> #endif
>>
>> Why have the #if at all?
>>
>>
>>
>
> Hmm... some old version compilers do not define __linux__ automatically
> (e.g. or32-linux-gcc 4.5.1-or32-1.0rc1, which is the latest cross
> compiler for openrisc, though).
>
> If not define __linux__, the compiler will report error (u_quad_t is
> not defined).
>
> When we remove "#if defined(__linux__)" from "include/linux/coda.h",
> the compiler will not report error, and 'cdev_t' will be defined as
> 'dev_t', not 'u_quad_t' (I guess, it is an implicit bug).
>
>
> In "uapi/include/*", only "coda.h" has "#if defined(__linux__)", maybe
> they want multiple OS can share the same "coda.h" file (at least, we
> can not say it is a bad idea).
>
> Neither suitable to define __linux__ in "include/linux/coda.h".
>
>
> All together, I think:
>
> the direct cause:
> "uapi/include/coda.h" wants to share itself for multiple OS (so they need check __linux__).
> (in "uapi/include/*", only coda.h need check __linux__)
>
> the root cause:
> the compiler for linux should define __linux__ automatically, the latest version of gcc has done, but some of the old version is not.
> most of cross compilers have merged their code into gcc main tree, but still left some (e.g. openrisc).
> (at least, I can not build openrisc from gcc main tree correctly, but most of others can)
> some of latest cross compilers still use old version of gcc, (still not define __linux__ automatically).
>
Maybe what I said is incorrect (it is just my current understanding).
Welcome any other members' suggestions or completions for discussing and
checking.
Thanks.
>
> The related code in "uapi/linux/coda.h" is below:
>
> -------------------------------code begin-------------------------------
>
> 73 #if defined(DJGPP) || defined(__CYGWIN32__)
> 74 #ifdef KERNEL
> 75 typedef unsigned long u_long;
> 76 typedef unsigned int u_int;
> 77 typedef unsigned short u_short;
> 78 typedef u_long ino_t;
> 79 typedef u_long dev_t;
> 80 typedef void * caddr_t;
> 81 #ifdef DOS
> 82 typedef unsigned __int64 u_quad_t;
> 83 #else
> 84 typedef unsigned long long u_quad_t;
> 85 #endif
> 86
> 87 #define inline
> 88
> 89 struct timespec {
> 90 long ts_sec;
> 91 long ts_nsec;
> 92 };
> 93 #else /* DJGPP but not KERNEL */
> 94 #include <sys/time.h>
> 95 typedef unsigned long long u_quad_t;
> 96 #endif /* !KERNEL */
> 97 #endif /* !DJGPP */
> 98
> 99
> 100 #if defined(__linux__)
> 101 #include <linux/time.h>
> 102 #define cdev_t u_quad_t
> 103 #ifndef __KERNEL__
> 104 #if !defined(_UQUAD_T_) && (!defined(__GLIBC__) || __GLIBC__ < 2)
> 105 #define _UQUAD_T_ 1
> 106 typedef unsigned long long u_quad_t;
> 107 #endif
> 108 #endif /* __KERNEL__ */
> 109 #else
> 110 #define cdev_t dev_t
> 111 #endif
> 112
>
> -------------------------------code end---------------------------------
>
>
> Thanks.
>
--
Chen Gang
On 07/31/2013 09:53 AM, Chen Gang wrote:
> On 07/31/2013 09:44 AM, Chen Gang wrote:
>> On 07/30/2013 08:29 PM, Joe Perches wrote:
>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>>> '#else' is useless, need remove.
>>>>
>>>> Signed-off-by: Chen Gang <[email protected]>
>>>> ---
>>>> include/linux/coda.h | 1 -
>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>>> index cff544f..d30209b 100644
>>>> --- a/include/linux/coda.h
>>>> +++ b/include/linux/coda.h
>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>>
>>>> #if defined(__linux__)
>>>> typedef unsigned long long u_quad_t;
>>>> -#else
>>>> #endif
>>>> #include <uapi/linux/coda.h>
>>>> #endif
>>>
>>> Why have the #if at all?
>>>
>>>
>>>
>>
OH, sorry, what I said about openrisc cross-compiler is not precise.
Need a patch for openrisc (just like another architectures have done):
-------------------------------diff begin-------------------------------
diff --git a/arch/openrisc/Makefile b/arch/openrisc/Makefile
index 4739b83..89076a6 100644
--- a/arch/openrisc/Makefile
+++ b/arch/openrisc/Makefile
@@ -24,7 +24,7 @@ OBJCOPYFLAGS := -O binary -R .note -R .comment -S
LDFLAGS_vmlinux :=
LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name)
-KBUILD_CFLAGS += -pipe -ffixed-r10
+KBUILD_CFLAGS += -pipe -ffixed-r10 -D__linux__
ifeq ($(CONFIG_OPENRISC_HAVE_INST_MUL),y)
KBUILD_CFLAGS += $(call cc-option,-mhard-mul)
-------------------------------diff end---------------------------------
But for "include/linux/coda.h", I still suggest to check __linux__
whether defined, at least it can find the building issues.
And next, I will send the related patch to openrisc mailing list.
:-)
Thanks.
>> Hmm... some old version compilers do not define __linux__ automatically
>> (e.g. or32-linux-gcc 4.5.1-or32-1.0rc1, which is the latest cross
>> compiler for openrisc, though).
>>
>> If not define __linux__, the compiler will report error (u_quad_t is
>> not defined).
>>
>> When we remove "#if defined(__linux__)" from "include/linux/coda.h",
>> the compiler will not report error, and 'cdev_t' will be defined as
>> 'dev_t', not 'u_quad_t' (I guess, it is an implicit bug).
>>
>>
>> In "uapi/include/*", only "coda.h" has "#if defined(__linux__)", maybe
>> they want multiple OS can share the same "coda.h" file (at least, we
>> can not say it is a bad idea).
>>
>> Neither suitable to define __linux__ in "include/linux/coda.h".
>>
>>
>> All together, I think:
>>
>> the direct cause:
>> "uapi/include/coda.h" wants to share itself for multiple OS (so they need check __linux__).
>> (in "uapi/include/*", only coda.h need check __linux__)
>>
>> the root cause:
>> the compiler for linux should define __linux__ automatically, the latest version of gcc has done, but some of the old version is not.
>> most of cross compilers have merged their code into gcc main tree, but still left some (e.g. openrisc).
>> (at least, I can not build openrisc from gcc main tree correctly, but most of others can)
>> some of latest cross compilers still use old version of gcc, (still not define __linux__ automatically).
>>
>
> Maybe what I said is incorrect (it is just my current understanding).
>
> Welcome any other members' suggestions or completions for discussing and
> checking.
>
> Thanks.
>
>>
>> The related code in "uapi/linux/coda.h" is below:
>>
>> -------------------------------code begin-------------------------------
>>
>> 73 #if defined(DJGPP) || defined(__CYGWIN32__)
>> 74 #ifdef KERNEL
>> 75 typedef unsigned long u_long;
>> 76 typedef unsigned int u_int;
>> 77 typedef unsigned short u_short;
>> 78 typedef u_long ino_t;
>> 79 typedef u_long dev_t;
>> 80 typedef void * caddr_t;
>> 81 #ifdef DOS
>> 82 typedef unsigned __int64 u_quad_t;
>> 83 #else
>> 84 typedef unsigned long long u_quad_t;
>> 85 #endif
>> 86
>> 87 #define inline
>> 88
>> 89 struct timespec {
>> 90 long ts_sec;
>> 91 long ts_nsec;
>> 92 };
>> 93 #else /* DJGPP but not KERNEL */
>> 94 #include <sys/time.h>
>> 95 typedef unsigned long long u_quad_t;
>> 96 #endif /* !KERNEL */
>> 97 #endif /* !DJGPP */
>> 98
>> 99
>> 100 #if defined(__linux__)
>> 101 #include <linux/time.h>
>> 102 #define cdev_t u_quad_t
>> 103 #ifndef __KERNEL__
>> 104 #if !defined(_UQUAD_T_) && (!defined(__GLIBC__) || __GLIBC__ < 2)
>> 105 #define _UQUAD_T_ 1
>> 106 typedef unsigned long long u_quad_t;
>> 107 #endif
>> 108 #endif /* __KERNEL__ */
>> 109 #else
>> 110 #define cdev_t dev_t
>> 111 #endif
>> 112
>>
>> -------------------------------code end---------------------------------
>>
>>
>> Thanks.
>>
>
>
--
Chen Gang
On 07/31/2013 11:16 AM, Chen Gang F T wrote:
> On 07/31/2013 09:53 AM, Chen Gang wrote:
>> On 07/31/2013 09:44 AM, Chen Gang wrote:
>>> On 07/30/2013 08:29 PM, Joe Perches wrote:
>>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>>>> '#else' is useless, need remove.
>>>>>
>>>>> Signed-off-by: Chen Gang <[email protected]>
>>>>> ---
>>>>> include/linux/coda.h | 1 -
>>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>>>> index cff544f..d30209b 100644
>>>>> --- a/include/linux/coda.h
>>>>> +++ b/include/linux/coda.h
>>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>>>
>>>>> #if defined(__linux__)
>>>>> typedef unsigned long long u_quad_t;
>>>>> -#else
>>>>> #endif
>>>>> #include <uapi/linux/coda.h>
>>>>> #endif
>>>>
>>>> Why have the #if at all?
>>>>
Hmm... if any members have some questions about the fix, commonly it
means the related fix need additional improvement.
Is it better to use BUILD_BUG() for the fix ? e.g. the diff may like
below (based on the applied patch):
-----------------------------diff begin----------------------------
diff --git a/include/linux/coda.h b/include/linux/coda.h
index d30209b..5f87d5d 100644
--- a/include/linux/coda.h
+++ b/include/linux/coda.h
@@ -60,6 +60,11 @@ Mellon the rights to redistribute these changes without encumbrance.
#if defined(__linux__)
typedef unsigned long long u_quad_t;
-#endif
+#else
+#include <linux/bug.h>
+BUILD_BUG();
+#endif /* !__linux__ */
+
#include <uapi/linux/coda.h>
+
#endif
-----------------------------diff end------------------------------
Thanks.
>>>>
>>>>
>>>
>
> OH, sorry, what I said about openrisc cross-compiler is not precise.
>
> Need a patch for openrisc (just like another architectures have done):
>
> -------------------------------diff begin-------------------------------
>
> diff --git a/arch/openrisc/Makefile b/arch/openrisc/Makefile
> index 4739b83..89076a6 100644
> --- a/arch/openrisc/Makefile
> +++ b/arch/openrisc/Makefile
> @@ -24,7 +24,7 @@ OBJCOPYFLAGS := -O binary -R .note -R .comment -S
> LDFLAGS_vmlinux :=
> LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name)
>
> -KBUILD_CFLAGS += -pipe -ffixed-r10
> +KBUILD_CFLAGS += -pipe -ffixed-r10 -D__linux__
>
> ifeq ($(CONFIG_OPENRISC_HAVE_INST_MUL),y)
> KBUILD_CFLAGS += $(call cc-option,-mhard-mul)
>
> -------------------------------diff end---------------------------------
>
> But for "include/linux/coda.h", I still suggest to check __linux__
> whether defined, at least it can find the building issues.
>
> And next, I will send the related patch to openrisc mailing list.
>
> :-)
>
> Thanks.
>
>>> Hmm... some old version compilers do not define __linux__ automatically
>>> (e.g. or32-linux-gcc 4.5.1-or32-1.0rc1, which is the latest cross
>>> compiler for openrisc, though).
>>>
>>> If not define __linux__, the compiler will report error (u_quad_t is
>>> not defined).
>>>
>>> When we remove "#if defined(__linux__)" from "include/linux/coda.h",
>>> the compiler will not report error, and 'cdev_t' will be defined as
>>> 'dev_t', not 'u_quad_t' (I guess, it is an implicit bug).
>>>
>>>
>>> In "uapi/include/*", only "coda.h" has "#if defined(__linux__)", maybe
>>> they want multiple OS can share the same "coda.h" file (at least, we
>>> can not say it is a bad idea).
>>>
>>> Neither suitable to define __linux__ in "include/linux/coda.h".
>>>
>>>
>>> All together, I think:
>>>
>>> the direct cause:
>>> "uapi/include/coda.h" wants to share itself for multiple OS (so they need check __linux__).
>>> (in "uapi/include/*", only coda.h need check __linux__)
>>>
>>> the root cause:
>>> the compiler for linux should define __linux__ automatically, the latest version of gcc has done, but some of the old version is not.
>>> most of cross compilers have merged their code into gcc main tree, but still left some (e.g. openrisc).
>>> (at least, I can not build openrisc from gcc main tree correctly, but most of others can)
>>> some of latest cross compilers still use old version of gcc, (still not define __linux__ automatically).
>>>
>>
>> Maybe what I said is incorrect (it is just my current understanding).
>>
>> Welcome any other members' suggestions or completions for discussing and
>> checking.
>>
>> Thanks.
>>
>>>
>>> The related code in "uapi/linux/coda.h" is below:
>>>
>>> -------------------------------code begin-------------------------------
>>>
>>> 73 #if defined(DJGPP) || defined(__CYGWIN32__)
>>> 74 #ifdef KERNEL
>>> 75 typedef unsigned long u_long;
>>> 76 typedef unsigned int u_int;
>>> 77 typedef unsigned short u_short;
>>> 78 typedef u_long ino_t;
>>> 79 typedef u_long dev_t;
>>> 80 typedef void * caddr_t;
>>> 81 #ifdef DOS
>>> 82 typedef unsigned __int64 u_quad_t;
>>> 83 #else
>>> 84 typedef unsigned long long u_quad_t;
>>> 85 #endif
>>> 86
>>> 87 #define inline
>>> 88
>>> 89 struct timespec {
>>> 90 long ts_sec;
>>> 91 long ts_nsec;
>>> 92 };
>>> 93 #else /* DJGPP but not KERNEL */
>>> 94 #include <sys/time.h>
>>> 95 typedef unsigned long long u_quad_t;
>>> 96 #endif /* !KERNEL */
>>> 97 #endif /* !DJGPP */
>>> 98
>>> 99
>>> 100 #if defined(__linux__)
>>> 101 #include <linux/time.h>
>>> 102 #define cdev_t u_quad_t
>>> 103 #ifndef __KERNEL__
>>> 104 #if !defined(_UQUAD_T_) && (!defined(__GLIBC__) || __GLIBC__ < 2)
>>> 105 #define _UQUAD_T_ 1
>>> 106 typedef unsigned long long u_quad_t;
>>> 107 #endif
>>> 108 #endif /* __KERNEL__ */
>>> 109 #else
>>> 110 #define cdev_t dev_t
>>> 111 #endif
>>> 112
>>>
>>> -------------------------------code end---------------------------------
>>>
>>>
>>> Thanks.
>>>
>>
>>
>
>
--
Chen Gang
On Mon, 2013-08-05 at 08:24 +0800, Chen Gang wrote:
> On 07/31/2013 11:16 AM, Chen Gang F T wrote:
> > On 07/31/2013 09:53 AM, Chen Gang wrote:
> >> On 07/31/2013 09:44 AM, Chen Gang wrote:
> >>> On 07/30/2013 08:29 PM, Joe Perches wrote:
> >>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
> >>>>> '#else' is useless, need remove.
> >>>>>
> >>>>> Signed-off-by: Chen Gang <[email protected]>
> >>>>> ---
> >>>>> include/linux/coda.h | 1 -
> >>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
> >>>>> index cff544f..d30209b 100644
> >>>>> --- a/include/linux/cod
> >>>>> +++ b/include/linux/coda.h
> >>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
> >>>>>
> >>>>> #if defined(__linux__)
> >>>>> typedef unsigned long long u_quad_t;
> >>>>> -#else
> >>>>> #endif
> >>>>> #include <uapi/linux/coda.h>
> >>>>> #endif
> >>>>
> >>>> Why have the #if at all?
> >>>>
>
> Hmm... if any members have some questions about the fix, commonly it
> means the related fix need additional improvement.
>
> Is it better to use BUILD_BUG() for the fix ?
Probably not. __linux__ is predefined by gcc.
Using an #ifdef __linux__ might make some sense in
in include/uapi, but not in include/linux.
On 08/05/2013 09:21 AM, Joe Perches wrote:
> On Mon, 2013-08-05 at 08:24 +0800, Chen Gang wrote:
>> On 07/31/2013 11:16 AM, Chen Gang F T wrote:
>>> On 07/31/2013 09:53 AM, Chen Gang wrote:
>>>> On 07/31/2013 09:44 AM, Chen Gang wrote:
>>>>> On 07/30/2013 08:29 PM, Joe Perches wrote:
>>>>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>>>>>> '#else' is useless, need remove.
>>>>>>>
>>>>>>> Signed-off-by: Chen Gang <[email protected]>
>>>>>>> ---
>>>>>>> include/linux/coda.h | 1 -
>>>>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>>>>>> index cff544f..d30209b 100644
>>>>>>> --- a/include/linux/cod
>>>>>>> +++ b/include/linux/coda.h
>>>>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>>>>>
>>>>>>> #if defined(__linux__)
>>>>>>> typedef unsigned long long u_quad_t;
>>>>>>> -#else
>>>>>>> #endif
>>>>>>> #include <uapi/linux/coda.h>
>>>>>>> #endif
>>>>>>
>>>>>> Why have the #if at all?
>>>>>>
>>
>> Hmm... if any members have some questions about the fix, commonly it
>> means the related fix need additional improvement.
>>
>> Is it better to use BUILD_BUG() for the fix ?
>
> Probably not. __linux__ is predefined by gcc.
>
> Using an #ifdef __linux__ might make some sense in
> in include/uapi, but not in include/linux.
>
>
Hmm... since "linux/coda.h" will include "uapi/linux/coda.h", it is
necessary to check __linux__ whether defined.
For some old compilers which is still widely used, they really do not
define __linux__, when use these compilers, we need give a necessary
check, if can not pass, need report BUILD_BUG().
e.g. for some versions of openrisc, please ref:
https://lkml.org/lkml/2013/7/30/888. (it is in applying work flow).
Thanks.
--
Chen Gang
On 08/05/2013 09:53 AM, Chen Gang wrote:
> On 08/05/2013 09:21 AM, Joe Perches wrote:
>> On Mon, 2013-08-05 at 08:24 +0800, Chen Gang wrote:
>>> On 07/31/2013 11:16 AM, Chen Gang F T wrote:
>>>> On 07/31/2013 09:53 AM, Chen Gang wrote:
>>>>> On 07/31/2013 09:44 AM, Chen Gang wrote:
>>>>>> On 07/30/2013 08:29 PM, Joe Perches wrote:
>>>>>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>>>>>>> '#else' is useless, need remove.
>>>>>>>>
>>>>>>>> Signed-off-by: Chen Gang <[email protected]>
>>>>>>>> ---
>>>>>>>> include/linux/coda.h | 1 -
>>>>>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>>>>>>> index cff544f..d30209b 100644
>>>>>>>> --- a/include/linux/cod
>>>>>>>> +++ b/include/linux/coda.h
>>>>>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>>>>>>
>>>>>>>> #if defined(__linux__)
>>>>>>>> typedef unsigned long long u_quad_t;
>>>>>>>> -#else
>>>>>>>> #endif
>>>>>>>> #include <uapi/linux/coda.h>
>>>>>>>> #endif
>>>>>>>
>>>>>>> Why have the #if at all?
>>>>>>>
>>>
>>> Hmm... if any members have some questions about the fix, commonly it
>>> means the related fix need additional improvement.
>>>
>>> Is it better to use BUILD_BUG() for the fix ?
>>
>> Probably not. __linux__ is predefined by gcc.
>>
>> Using an #ifdef __linux__ might make some sense in
>> in include/uapi, but not in include/linux.
>>
>>
>
In fact, in my opinion "uapi/linux/coda.h" should not content "#ifdef
__linux__", since it is already in "uapi/linux/" sub-directory.
If it still wants to universal enough to multiple OS, need move it from
"uapi/linux/coda.h" to "uapi/coda/coda.h".
:-)
> Hmm... since "linux/coda.h" will include "uapi/linux/coda.h", it is
> necessary to check __linux__ whether defined.
>
> For some old compilers which is still widely used, they really do not
> define __linux__, when use these compilers, we need give a necessary
> check, if can not pass, need report BUILD_BUG().
>
> e.g. for some versions of openrisc, please ref:
> https://lkml.org/lkml/2013/7/30/888. (it is in applying work flow).
>
>
> Thanks.
>
--
Chen Gang
On 08/05/2013 10:12 AM, Chen Gang wrote:
> On 08/05/2013 09:53 AM, Chen Gang wrote:
>> On 08/05/2013 09:21 AM, Joe Perches wrote:
>>> On Mon, 2013-08-05 at 08:24 +0800, Chen Gang wrote:
>>>> On 07/31/2013 11:16 AM, Chen Gang F T wrote:
>>>>> On 07/31/2013 09:53 AM, Chen Gang wrote:
>>>>>> On 07/31/2013 09:44 AM, Chen Gang wrote:
>>>>>>> On 07/30/2013 08:29 PM, Joe Perches wrote:
>>>>>>>> On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
>>>>>>>>> '#else' is useless, need remove.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Chen Gang <[email protected]>
>>>>>>>>> ---
>>>>>>>>> include/linux/coda.h | 1 -
>>>>>>>>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/include/linux/coda.h b/include/linux/coda.h
>>>>>>>>> index cff544f..d30209b 100644
>>>>>>>>> --- a/include/linux/cod
>>>>>>>>> +++ b/include/linux/coda.h
>>>>>>>>> @@ -60,7 +60,6 @@ Mellon the rights to redistribute these changes without encumbrance.
>>>>>>>>>
>>>>>>>>> #if defined(__linux__)
>>>>>>>>> typedef unsigned long long u_quad_t;
>>>>>>>>> -#else
>>>>>>>>> #endif
>>>>>>>>> #include <uapi/linux/coda.h>
>>>>>>>>> #endif
>>>>>>>>
>>>>>>>> Why have the #if at all?
>>>>>>>>
>>>>
>>>> Hmm... if any members have some questions about the fix, commonly it
>>>> means the related fix need additional improvement.
>>>>
>>>> Is it better to use BUILD_BUG() for the fix ?
>>>
>>> Probably not. __linux__ is predefined by gcc.
>>>
>>> Using an #ifdef __linux__ might make some sense in
>>> in include/uapi, but not in include/linux.
>>>
>>>
>>
In "include/uapi/", only 2 files contant __linux__: (especially latest
compiler has already defined __linux__)
[root@gchenlinux uapi]# grep -rn __linux__ *
drm/drm.h:39:#if defined(__KERNEL__) || defined(__linux__)
linux/coda.h:100:#if defined(__linux__)
So now and future, it is not common enough to check __linux__ globally.
So I still suggest to let the related modules theirselves to check
__linux__ whether defined.
>
> In fact, in my opinion "uapi/linux/coda.h" should not content "#ifdef
> __linux__", since it is already in "uapi/linux/" sub-directory.
>
> If it still wants to universal enough to multiple OS, need move it from
> "uapi/linux/coda.h" to "uapi/coda/coda.h".
>
> :-)
>
>
>> Hmm... since "linux/coda.h" will include "uapi/linux/coda.h", it is
>> necessary to check __linux__ whether defined.
>>
>> For some old compilers which is still widely used, they really do not
>> define __linux__, when use these compilers, we need give a necessary
>> check, if can not pass, need report BUILD_BUG().
>>
>> e.g. for some versions of openrisc, please ref:
>> https://lkml.org/lkml/2013/7/30/888. (it is in applying work flow).
>>
>>
>> Thanks.
>>
>
>
--
Chen Gang