diff mbox

netfilter: Fix compiler warning.

Message ID pvmein2bj1p.fsf@chavey.mtv.corp.google.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

laurent chavey Dec. 10, 2009, 9:44 p.m. UTC 
Fix compiler warning "discards qualifiers from pointer target type",
by allowing explicit discard of const qualifier thru type casting.
The const_cast() macro is taken from a patch from Kaveh R. Ghazi
[PATCH]: Fix problematic -Wcast-qual cases using new CONST_CAST macro
posted on 
http://old.nabble.com/-PATCH-:-Fix-problematic--Wcast-qual-cases-using-new-CONST_CAST--macro-td11824676.html
The proposed __nowarn__ keyword has not yet been implemented, 
(see http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01261.html).


Signed-off-by: Laurent Chavey <chavey@google.com>
---
 include/linux/kernel.h        |   12 ++++++++++++
 net/ipv4/netfilter/ipt_ULOG.c |    2 +-
 net/netfilter/xt_time.c       |    2 +-
 3 files changed, 14 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jan Engelhardt Dec. 10, 2009, 10:06 p.m. UTC | #1 
On Donnerstag 2009-12-10 22:44, chavey@google.com wrote:

>Fix compiler warning "discards qualifiers from pointer target type",
>by allowing explicit discard of const qualifier thru type casting.
>The const_cast() macro is taken from a patch from Kaveh R. Ghazi

Uhm, I am not getting any warning on xt_time with either gcc-4.4.1
or gcc-4.5_20091126, so what case are we actually trying to fix?

>--- a/net/netfilter/xt_time.c
>+++ b/net/netfilter/xt_time.c
>@@ -170,7 +170,7 @@ time_mt(const struct sk_buff *skb, const struct xt_match_param *par)
> 	 * it arrived at the right moment before 13:00.
> 	 */
> 	if (skb->tstamp.tv64 == 0)
>-		__net_timestamp((struct sk_buff *)skb);
>+		__net_timestamp((struct sk_buff *)const_cast(skb));
> 
> 	stamp = ktime_to_ns(skb->tstamp);
> 	stamp = div_s64(stamp, NSEC_PER_SEC);
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Dec. 10, 2009, 10:08 p.m. UTC | #2 
From: Jan Engelhardt <jengelh@medozas.de>
Date: Thu, 10 Dec 2009 23:06:32 +0100 (CET)

> 
> On Donnerstag 2009-12-10 22:44, chavey@google.com wrote:
> 
>>Fix compiler warning "discards qualifiers from pointer target type",
>>by allowing explicit discard of const qualifier thru type casting.
>>The const_cast() macro is taken from a patch from Kaveh R. Ghazi
> 
> Uhm, I am not getting any warning on xt_time with either gcc-4.4.1
> or gcc-4.5_20091126, so what case are we actually trying to fix?

I get this warning with gcc-4.3.x all the time.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrew Morton Dec. 10, 2009, 10:13 p.m. UTC | #3 
On Thu, 10 Dec 2009 13:44:50 -0800
chavey@google.com wrote:

> Fix compiler warning "discards qualifiers from pointer target type",
> by allowing explicit discard of const qualifier thru type casting.
> The const_cast() macro is taken from a patch from Kaveh R. Ghazi
> [PATCH]: Fix problematic -Wcast-qual cases using new CONST_CAST macro
> posted on 
> http://old.nabble.com/-PATCH-:-Fix-problematic--Wcast-qual-cases-using-new-CONST_CAST--macro-td11824676.html
> The proposed __nowarn__ keyword has not yet been implemented, 
> (see http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01261.html).
> 
> 
> Signed-off-by: Laurent Chavey <chavey@google.com>
> ---
>  include/linux/kernel.h        |   12 ++++++++++++
>  net/ipv4/netfilter/ipt_ULOG.c |    2 +-
>  net/netfilter/xt_time.c       |    2 +-
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index 3fa4c59..0246771 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -710,4 +710,16 @@ struct sysinfo {
>  # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
>  #endif
>  
> +/* Cast away const-ness to pass -Wcast-qual */

This comment simply doesn't make sense.

I don't think it adequately explains why the macro exists, how and when
one should use it, etc.

> +#ifdef __GNUC__
> +union gcc_constcast
> +{

union gcc_constcast {

> +	const void *cv;
> +	void *v;
> +};
> +#define const_cast(X) ((__extension__(union gcc_constcast)(const void *)(X)).v)
> +#else
> +#define const_cast(X) ((void *)(X))
> +#endif

Why don't we just use the simple version on all compilers?  What's the
advantage in using the gcc-specific extension?

And I was wrong - this should be implemented in linux/compiler.h.  We
have infrastructure there to separate gcc-specific and non-gcc
implementations of the same interface.

Finally, are we sure that __extension__(union gcc_constcast) is
available on all gcc versions mentioned in Documentation/Changes?  If
not, this patch should use include/linux/compiler-gcc4.h, etc.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt Dec. 10, 2009, 11:23 p.m. UTC | #4 
On Donnerstag 2009-12-10 23:08, David Miller wrote:
>> 
>>>Fix compiler warning "discards qualifiers from pointer target type",
>>>by allowing explicit discard of const qualifier thru type casting.
>>>The const_cast() macro is taken from a patch from Kaveh R. Ghazi
>> 
>> Uhm, I am not getting any warning on xt_time with either gcc-4.4.1
>> or gcc-4.5_20091126, so what case are we actually trying to fix?
>
>I get this warning with gcc-4.3.x all the time.

Ok. Still, assuming it was a "bug" which got fixed in gcc-4.4,
should we really bother with it anymore? (Thankfully it's just
a warning, and not an error.)
If so, we can probably do better code-wise.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt Dec. 10, 2009, 11:44 p.m. UTC | #5 
On Donnerstag 2009-12-10 23:08, David Miller wrote:
>> On Donnerstag 2009-12-10 22:44, chavey@google.com wrote:
>> 
>>>Fix compiler warning "discards qualifiers from pointer target type",
>>>by allowing explicit discard of const qualifier thru type casting.
>>>The const_cast() macro is taken from a patch from Kaveh R. Ghazi
>> 
>> Uhm, I am not getting any warning on xt_time with either gcc-4.4.1
>> or gcc-4.5_20091126, so what case are we actually trying to fix?
>
>I get this warning with gcc-4.3.x all the time.

Seems they fixed it in gcc-4.3 too?

00:41 borg:~/code/linux > make net/netfilter/xt_time.o -j8 CC=gcc-4.3
  CHK     include/linux/version.h
  CHK     include/linux/utsrelease.h
  SYMLINK include/asm -> include/asm-x86
  CC      scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/modpost.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC      kernel/bounds.s
  GEN     include/linux/bounds.h
  CC      arch/x86/kernel/asm-offsets.s
  GEN     include/asm/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CC [M]  net/netfilter/xt_time.o
00:42 borg:~/code/linux > gcc-4.3 -v
Using built-in specs.
Target: x86_64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info 
--mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 
--enable-languages=c,c++,objc,fortran,obj-c++,java,ada 
--enable-checking=release --with-gxx-include-dir=/usr/include/c++/4.3 
--enable-ssp --disable-libssp --with-bugurl=http://bugs.opensuse.org/ 
--with-pkgversion='SUSE Linux' --disable-libgcj --disable-libmudflap 
--with-slibdir=/lib64 --with-system-zlib --enable-__cxa_atexit 
--enable-libstdcxx-allocator=new --disable-libstdcxx-pch 
--enable-version-specific-runtime-libs --program-suffix=-4.3 
--enable-linux-futex --without-system-libunwind --with-cpu=generic 
--build=x86_64-suse-linux
Thread model: posix
gcc version 4.3.4 (SUSE Linux)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy Dec. 14, 2009, 1:20 p.m. UTC | #6 
Jan Engelhardt wrote:
> On Donnerstag 2009-12-10 23:08, David Miller wrote:
>>> On Donnerstag 2009-12-10 22:44, chavey@google.com wrote:
>>>
>>>> Fix compiler warning "discards qualifiers from pointer target type",
>>>> by allowing explicit discard of const qualifier thru type casting.
>>>> The const_cast() macro is taken from a patch from Kaveh R. Ghazi
>>> Uhm, I am not getting any warning on xt_time with either gcc-4.4.1
>>> or gcc-4.5_20091126, so what case are we actually trying to fix?
>> I get this warning with gcc-4.3.x all the time.
> 
> Seems they fixed it in gcc-4.3 too?
> 
> 00:41 borg:~/code/linux > make net/netfilter/xt_time.o -j8 CC=gcc-4.3

Yes, I haven't seen it for a long time either, currently using
gcc version 4.3.4 (Debian 4.3.4-6) and gcc version 4.4.1
(Debian 4.4.1-3)

I don't think we need to bother with this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fa4c59..0246771 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -710,4 +710,16 @@  struct sysinfo {
 # define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
 #endif
 
+/* Cast away const-ness to pass -Wcast-qual */
+#ifdef __GNUC__
+union gcc_constcast
+{
+	const void *cv;
+	void *v;
+};
+#define const_cast(X) ((__extension__(union gcc_constcast)(const void *)(X)).v)
+#else
+#define const_cast(X) ((void *)(X))
+#endif
+
 #endif
diff --git a/net/ipv4/netfilter/ipt_ULOG.c b/net/ipv4/netfilter/ipt_ULOG.c
index d32cc4b..c8f8b8b 100644
--- a/net/ipv4/netfilter/ipt_ULOG.c
+++ b/net/ipv4/netfilter/ipt_ULOG.c
@@ -210,7 +210,7 @@  static void ipt_ulog_packet(unsigned int hooknum,
 
 	/* We might not have a timestamp, get one */
 	if (skb->tstamp.tv64 == 0)
-		__net_timestamp((struct sk_buff *)skb);
+		__net_timestamp((struct sk_buff *)const_cast(skb));
 
 	/* copy hook, prefix, timestamp, payload, etc. */
 	pm->data_len = copy_len;
diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index 93acaa5..8f34a4a 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -170,7 +170,7 @@  time_mt(const struct sk_buff *skb, const struct xt_match_param *par)
 	 * it arrived at the right moment before 13:00.
 	 */
 	if (skb->tstamp.tv64 == 0)
-		__net_timestamp((struct sk_buff *)skb);
+		__net_timestamp((struct sk_buff *)const_cast(skb));
 
 	stamp = ktime_to_ns(skb->tstamp);
 	stamp = div_s64(stamp, NSEC_PER_SEC);