Re: [PATCH] INET: fix incorrect inet_sock-is_icsk assignment

2007-01-10 Thread Jarek Poplawski
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

2007-01-10 Thread Paul Moore
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

2007-01-10 Thread Jarek Poplawski
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

2007-01-09 Thread Jarek Poplawski
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

2007-01-09 Thread Paul Moore
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

2007-01-09 Thread Arnaldo Carvalho de Melo

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

2007-01-08 Thread Jarek Poplawski
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

2007-01-08 Thread Paul Moore
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

2007-01-04 Thread David Miller
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

2007-01-04 Thread Arnaldo Carvalho de Melo

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

2007-01-04 Thread David Miller
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