Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Tue, Jan 09, 2007 at 09:26:46AM -0500, Paul Moore wrote: On Tuesday 09 January 2007 3:43 am, Jarek Poplawski wrote: ... But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... This from the guy who believes Justin Timberlake rocks! ;) All right, you convinced me, I'll send out a patch to the patch later today. I'm terribly sorry, I didn't expected any messing now! I rather thought about some preferences for the future. Justin really rocks (now), in particular, as an example to the popular HOWTO. Jennifer Lopez, Snoop Dogg or 50 Cent as well. But for an IEEE document: Pink Floyd, Led Zeppelin or Jimi Hendrix would be more appropriate, probably, and obviously they also rock more! Cheers, Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Wednesday 10 January 2007 5:01 am, Jarek Poplawski wrote: On Tue, Jan 09, 2007 at 09:26:46AM -0500, Paul Moore wrote: On Tuesday 09 January 2007 3:43 am, Jarek Poplawski wrote: ... But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... This from the guy who believes Justin Timberlake rocks! ;) All right, you convinced me, I'll send out a patch to the patch later today. I'm terribly sorry, I didn't expected any messing now! I rather thought about some preferences for the future. No need to apologize, it was a trivial fix and I figured I might as well take care of now ... however, some of your taste in music does require an apology ;) -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Wed, Jan 10, 2007 at 08:13:57AM -0500, Paul Moore wrote: On Wednesday 10 January 2007 5:01 am, Jarek Poplawski wrote: On Tue, Jan 09, 2007 at 09:26:46AM -0500, Paul Moore wrote: On Tuesday 09 January 2007 3:43 am, Jarek Poplawski wrote: ... But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... This from the guy who believes Justin Timberlake rocks! ;) All right, you convinced me, I'll send out a patch to the patch later today. I'm terribly sorry, I didn't expected any messing now! I rather thought about some preferences for the future. No need to apologize, it was a trivial fix and I figured I might as well take care of now ... however, some of your taste in music does require an apology ;) All right, you convinced me too. I change 50 Cent to Busta Rhymes! Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Mon, Jan 08, 2007 at 09:47:29AM -0500, Paul Moore wrote: ... I guess it all depends on who is reading it ;) Sure! I only had a feeling your way is maybe slightly less often used so I wanted some opinion. Personally, I don't care too much either way as long as it is fixed. Yes, you've done great work, no doubt. But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... Cheers, Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Tuesday 09 January 2007 3:43 am, Jarek Poplawski wrote: ... But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... This from the guy who believes Justin Timberlake rocks! ;) All right, you convinced me, I'll send out a patch to the patch later today. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On 1/9/07, Paul Moore [EMAIL PROTECTED] wrote: On Tuesday 09 January 2007 3:43 am, Jarek Poplawski wrote: ... But if you consider this code will probably become classical and will be read, quoted and teached next 1000 years, then the style could matter... This from the guy who believes Justin Timberlake rocks! ;) All right, you convinced me, I'll send out a patch to the patch later today. /me grins I see I don't have to worry that much with style nitpicking, keep it up! - Arnaldo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On 04-01-2007 21:04, Paul Moore wrote: ... +++ net-2.6.20_bugfix_2/net/ipv4/af_inet.c @@ -305,7 +305,7 @@ lookup_protocol: sk-sk_reuse = 1; inet = inet_sk(sk); - inet-is_icsk = INET_PROTOSW_ICSK answer_flags; + inet-is_icsk = (INET_PROTOSW_ICSK answer_flags) == INET_PROTOSW_ICSK; Isn't this more readable like this?: inet-is_icsk = (INET_PROTOSW_ICSK answer_flags) != 0; Regards, Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On Monday, January 8 2007 8:25 am, Jarek Poplawski wrote: On 04-01-2007 21:04, Paul Moore wrote: +++ net-2.6.20_bugfix_2/net/ipv4/af_inet.c @@ -305,7 +305,7 @@ lookup_protocol: sk-sk_reuse = 1; inet = inet_sk(sk); - inet-is_icsk = INET_PROTOSW_ICSK answer_flags; + inet-is_icsk = (INET_PROTOSW_ICSK answer_flags) == INET_PROTOSW_ICSK; Isn't this more readable like this?: inet-is_icsk = (INET_PROTOSW_ICSK answer_flags) != 0; I guess it all depends on who is reading it ;) Personally, I don't care too much either way as long as it is fixed. -- paul moore linux security @ hp - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
From: Paul Moore [EMAIL PROTECTED] Date: Thu, 04 Jan 2007 15:04:31 -0500 From: Paul Moore [EMAIL PROTECTED] The inet_create() and inet6_create() functions incorrectly set the inet_sock-is_icsk field. Both functions assume that the is_icsk field is large enough to hold at least a INET_PROTOSW_ICSK value when it is actually only a single bit. This patch corrects the assignment by doing a boolean comparison whose result will safely fit into a single bit field. Signed-off-by: Paul Moore [EMAIL PROTECTED] Applied, thanks a lot Paul. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
On 1/4/07, David Miller [EMAIL PROTECTED] wrote: From: Paul Moore [EMAIL PROTECTED] Date: Thu, 04 Jan 2007 15:04:31 -0500 From: Paul Moore [EMAIL PROTECTED] The inet_create() and inet6_create() functions incorrectly set the inet_sock-is_icsk field. Both functions assume that the is_icsk field is large enough to hold at least a INET_PROTOSW_ICSK value when it is actually only a single bit. This patch corrects the assignment by doing a boolean comparison whose result will safely fit into a single bit field. Signed-off-by: Paul Moore [EMAIL PROTECTED] Applied, thanks a lot Paul. Well spotted, gcc let me down on this one: [EMAIL PROTECTED] ~]$ cat a.c #include stdio.h struct foo { int a_bit:1; }; int main(void) { int trickme = 0x04; struct foo oof; oof.a_bit = trickme 4; printf(%u\n, oof.a_bit); } [EMAIL PROTECTED] ~]$ make a cc a.c -o a [EMAIL PROTECTED] ~]$ ./a 0 [EMAIL PROTECTED] ~]$ But... [EMAIL PROTECTED] ~]$ cat a.c #include stdio.h struct foo { int a_bit:1; }; int main(void) { struct foo oof; oof.a_bit = 0x04; printf(%u\n, oof.a_bit); } [EMAIL PROTECTED] ~]$ make a cc a.c -o a a.c: In function 'main': a.c:8: warning: overflow in implicit constant conversion [EMAIL PROTECTED] ~]$ I expected a warning since the and operation clearly could yield a value that would overflow, just like in the constant case... Anyway, thanks a lot Paul! - Arnaldo - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment
From: Arnaldo Carvalho de Melo [EMAIL PROTECTED] Date: Fri, 5 Jan 2007 00:32:31 -0200 I expected a warning since the and operation clearly could yield a value that would overflow, just like in the constant case... It sounds stupid, but once you introduce variables and not everything is constance, GCC currently can't make the analysis. It's an interesting class of bugs to detect automatically, for sure :-) - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html