public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH trivial] include/linux/coda.h: remove useless '#else'
@ 2013-07-30  7:30 Chen Gang
  2013-07-30 10:38 ` Jiri Kosina
  2013-07-30 12:29 ` Joe Perches
  0 siblings, 2 replies; 12+ messages in thread
From: Chen Gang @ 2013-07-30  7:30 UTC (permalink / raw)
  To: 'Jiri Kosina'
  Cc: jaharkes, coda, codalist, linux-kernel@vger.kernel.org

'#else' is useless, need remove.

Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-30  7:30 [PATCH trivial] include/linux/coda.h: remove useless '#else' Chen Gang
@ 2013-07-30 10:38 ` Jiri Kosina
  2013-07-31  0:06   ` Chen Gang
  2013-07-30 12:29 ` Joe Perches
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Kosina @ 2013-07-30 10:38 UTC (permalink / raw)
  To: Chen Gang; +Cc: jaharkes, coda, codalist, linux-kernel@vger.kernel.org

On Tue, 30 Jul 2013, Chen Gang wrote:

> '#else' is useless, need remove.
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-30  7:30 [PATCH trivial] include/linux/coda.h: remove useless '#else' Chen Gang
  2013-07-30 10:38 ` Jiri Kosina
@ 2013-07-30 12:29 ` Joe Perches
  2013-07-31  1:44   ` Chen Gang
  1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-07-30 12:29 UTC (permalink / raw)
  To: Chen Gang
  Cc: 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

On Tue, 2013-07-30 at 15:30 +0800, Chen Gang wrote:
> '#else' is useless, need remove.
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  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?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-30 10:38 ` Jiri Kosina
@ 2013-07-31  0:06   ` Chen Gang
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Gang @ 2013-07-31  0:06 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: jaharkes, coda, codalist, linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-30 12:29 ` Joe Perches
@ 2013-07-31  1:44   ` Chen Gang
  2013-07-31  1:53     ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-31  1:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-31  1:44   ` Chen Gang
@ 2013-07-31  1:53     ` Chen Gang
  2013-07-31  3:16       ` Chen Gang F T
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-31  1:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>> ---
>>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-31  1:53     ` Chen Gang
@ 2013-07-31  3:16       ` Chen Gang F T
  2013-08-05  0:24         ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang F T @ 2013-07-31  3:16 UTC (permalink / raw)
  To: Chen Gang
  Cc: Joe Perches, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>>> ---
>>>>  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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-07-31  3:16       ` Chen Gang F T
@ 2013-08-05  0:24         ` Chen Gang
  2013-08-05  1:21           ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-08-05  0:24 UTC (permalink / raw)
  To: Chen Gang F T
  Cc: Joe Perches, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>>>> ---
>>>>>  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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-08-05  0:24         ` Chen Gang
@ 2013-08-05  1:21           ` Joe Perches
  2013-08-05  1:53             ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-08-05  1:21 UTC (permalink / raw)
  To: Chen Gang
  Cc: Chen Gang F T, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
> >>>>> ---
> >>>>>  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.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-08-05  1:21           ` Joe Perches
@ 2013-08-05  1:53             ` Chen Gang
  2013-08-05  2:12               ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-08-05  1:53 UTC (permalink / raw)
  To: Joe Perches
  Cc: Chen Gang F T, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>>>>>> ---
>>>>>>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-08-05  1:53             ` Chen Gang
@ 2013-08-05  2:12               ` Chen Gang
  2013-08-05  9:00                 ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-08-05  2:12 UTC (permalink / raw)
  To: Joe Perches
  Cc: Chen Gang F T, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>>>>>>> ---
>>>>>>>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
  2013-08-05  2:12               ` Chen Gang
@ 2013-08-05  9:00                 ` Chen Gang
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Gang @ 2013-08-05  9:00 UTC (permalink / raw)
  To: Joe Perches
  Cc: Chen Gang F T, 'Jiri Kosina', jaharkes, coda, codalist,
	linux-kernel@vger.kernel.org

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 <gang.chen@asianux.com>
>>>>>>>>> ---
>>>>>>>>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-08-05  9:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-30  7:30 [PATCH trivial] include/linux/coda.h: remove useless '#else' Chen Gang
2013-07-30 10:38 ` Jiri Kosina
2013-07-31  0:06   ` Chen Gang
2013-07-30 12:29 ` Joe Perches
2013-07-31  1:44   ` Chen Gang
2013-07-31  1:53     ` Chen Gang
2013-07-31  3:16       ` Chen Gang F T
2013-08-05  0:24         ` Chen Gang
2013-08-05  1:21           ` Joe Perches
2013-08-05  1:53             ` Chen Gang
2013-08-05  2:12               ` Chen Gang
2013-08-05  9:00                 ` Chen Gang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox