Re: [PATCH trivial] include/linux/coda.h: remove useless '#else'
From: Chen Gang
Date: Sun Aug 04 2013 - 20:25:28 EST
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@xxxxxxxxxxx>
>>>>> ---
>>>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/