Skip to contentSkip to conversation

drivers/net/acenic.c MIN/MAX removal

PATCH In review Michael Veeck 3 participants 7 messages Feb 7, 2004 – Apr 7, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/net/acenic.c MIN/MAX removal

Hi!
Patch (against 2.6.3-rc1) removes unnecessary min/max macros and changes calls to use kernel.h macros instead. Feedback always welcome
Michael

drivers/net/acenic.c +0 -4
@@ -335,10 +335,6 @@
 #define ACE_PROBE_ARG				struct net_device *dev
 #endif
 
-#ifndef min_t
-#define min_t(type,a,b)	(((a)<(b))?(a):(b))
-#endif
-
 #ifndef ARCH_HAS_PREFETCHW
 #ifndef prefetchw
 #define prefetchw(x)				do{} while(0)

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw
Quoted 12 lines · Michael Veeck

Hi!
Patch (against 2.6.3-rc1) removes unnecessary min/max macros and changes calls to use kernel.h macros instead.
Feedback always welcome
Michael

------------------------------------------------------------------------

drivers/net/acenic.c line 340

Quoted 6 lines
drivers/net/acenic.c +0 -3
@@ -335,10 +335,6 @@
 #define ACE_PROBE_ARG				struct net_device *dev
 #endif
 
-#ifndef min_t
-#define min_t(type,a,b)	(((a)<(b))?(a):(b))
-#endif

Read the code you're patching :)

You're killing compat code the driver author added.

Jeff

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Michael Veeck
Quoted 27 lines · Jeff Garzik
Quoted 14 lines · Michael Veeck

Hi!
Patch (against 2.6.3-rc1) removes unnecessary min/max macros and changes calls to use kernel.h macros instead.
Feedback always welcome
Michael

------------------------------------------------------------------------

--- linux-2.6.2.org/drivers/net/acenic.c 2004-02-07
15:25:57.000000000 +0100
+++ linux-2.6.2.new/drivers/net/acenic.c 2004-02-07
15:33:24.286033896 +0100

drivers/net/acenic.c line 340

Quoted 6 lines
drivers/net/acenic.c +0 -3
@@ -335,10 +335,6 @@
 #define ACE_PROBE_ARG                struct net_device *dev
 #endif
 
-#ifndef min_t
-#define min_t(type,a,b)    (((a)<(b))?(a):(b))
-#endif

Read the code you're patching :)

Quoted 4 lines

You're killing compat code the driver author added.

Jeff

Ah, that didnt came into my mind. But since #ifndef min_t doesnt seem to be in any of the "#if LINUX_VERSION_CODE >= " why not put it in the appropiate one?

Do you know when the min_t was introduced in include/linux/kernel.h ?

Veeck

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to Michael Veeck

On Sat, 07 Feb 2004 18:35:05 +0100 Michael Veeck <[email protected]> wrote:

| 
| 
| Jeff Garzik schrieb:
| > Michael Veeck wrote:
| > 
| >> Hi!
| >> Patch (against 2.6.3-rc1) removes unnecessary min/max macros and 
| >> changes calls to use kernel.h macros instead.
| >> Feedback always welcome
| >> Michael
| >>
| >>
| >> ------------------------------------------------------------------------
| >>
| >> --- linux-2.6.2.org/drivers/net/acenic.c    2004-02-07 
| >> 15:25:57.000000000 +0100
| >> +++ linux-2.6.2.new/drivers/net/acenic.c    2004-02-07 
| >> 15:33:24.286033896 +0100
| >> @@ -335,10 +335,6 @@
| >>  #define ACE_PROBE_ARG                struct net_device *dev
| >>  #endif
| >>  
| >> -#ifndef min_t
| >> -#define min_t(type,a,b)    (((a)<(b))?(a):(b))
| >> -#endif
| > 
| > 
| > 
| > Read the code you're patching :)
| > 
| > You're killing compat code the driver author added.
| > 
| >     Jeff
| > 
| 
| Ah, that didnt came into my mind. But since #ifndef min_t doesnt seem to 
| be in any of the "#if LINUX_VERSION_CODE >= " why not put it in the 
| appropiate one?
| 
| Do you know when the min_t was introduced in include/linux/kernel.h ?

BK shows 5-FEB-2002 for min_t.
That would be early 2.5.x, like 2.5.4, estimating by looking at
http://www.kernel.org/pub/linux/kernel/v2.5/

--
~Randy
kernel-janitors project: http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list [email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to Michael Veeck
Quoted 43 lines · Randy.Dunlap

On Sat, 07 Feb 2004 18:35:05 +0100 Michael Veeck <[email protected]> wrote:

| 
| 
| Jeff Garzik schrieb:
| > Michael Veeck wrote:
| > 
| >> Hi!
| >> Patch (against 2.6.3-rc1) removes unnecessary min/max macros and 
| >> changes calls to use kernel.h macros instead.
| >> Feedback always welcome
| >> Michael
| >>
| >>
| >> ------------------------------------------------------------------------
| >>
| >> --- linux-2.6.2.org/drivers/net/acenic.c    2004-02-07 
| >> 15:25:57.000000000 +0100
| >> +++ linux-2.6.2.new/drivers/net/acenic.c    2004-02-07 
| >> 15:33:24.286033896 +0100
| >> @@ -335,10 +335,6 @@
| >>  #define ACE_PROBE_ARG                struct net_device *dev
| >>  #endif
| >>  
| >> -#ifndef min_t
| >> -#define min_t(type,a,b)    (((a)<(b))?(a):(b))
| >> -#endif
| > 
| > 
| > 
| > Read the code you're patching :)
| > 
| > You're killing compat code the driver author added.
| > 
| >     Jeff
| > 
| 
| Ah, that didnt came into my mind. But since #ifndef min_t doesnt seem to 
| be in any of the "#if LINUX_VERSION_CODE >= " why not put it in the 
| appropiate one?
| 
| Do you know when the min_t was introduced in include/linux/kernel.h ?
Quoted 4 lines

BK shows 5-FEB-2002 for min_t.
That would be early 2.5.x, like 2.5.4, estimating by looking at
http://www.kernel.org/pub/linux/kernel/v2.5/

Digging through the changelogs on kernel.org I found out that min_t/max_t macros were really introduced in 2.4.9 as "min/max" and then changed to their min_t/max_t-name in 2.4.10.

I will incorproate that in my next patch! Thanks for the advice!

Veeck
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#5 lore raw
In reply to Michael Veeck
Quoted 38 lines · Michael Veeck
Quoted 27 lines · Randy.Dunlap

On Sat, 07 Feb 2004 18:35:05 +0100 Michael Veeck <[email protected]> wrote:

| | | Jeff Garzik schrieb:
| > Michael Veeck wrote:
| > | >> Hi!
| >> Patch (against 2.6.3-rc1) removes unnecessary min/max macros and | >> changes calls to use kernel.h macros instead.
| >> Feedback always welcome
| >> Michael
| >>
| >>
| >> 
------------------------------------------------------------------------
| >>
| >> --- linux-2.6.2.org/drivers/net/acenic.c    2004-02-07 | >> 15:25:57.000000000 +0100
| >> +++ linux-2.6.2.new/drivers/net/acenic.c    2004-02-07 | >> 15:33:24.286033896 +0100
| >> @@ -335,10 +335,6 @@
| >>  #define ACE_PROBE_ARG                struct net_device *dev
| >>  #endif
| >>  | >> -#ifndef min_t
| >> -#define min_t(type,a,b)    (((a)<(b))?(a):(b))
| >> -#endif
| > | > | > | > Read the code you're patching :)
| > | > You're killing compat code the driver author added.
| > | >     Jeff
| > | | Ah, that didnt came into my mind. But since #ifndef min_t doesnt seem to | be in any of the "#if LINUX_VERSION_CODE >= " why not put it in the | appropiate one?
| | Do you know when the min_t was introduced in include/linux/kernel.h ?
Quoted 4 lines

BK shows 5-FEB-2002 for min_t.
That would be early 2.5.x, like 2.5.4, estimating by looking at
http://www.kernel.org/pub/linux/kernel/v2.5/

Digging through the changelogs on kernel.org I found out that min_t/max_t macros were really introduced in 2.4.9 as "min/max" and then changed to their min_t/max_t-name in 2.4.10.

Quoted 9 lines

I will incorproate that in my next patch! Thanks for the advice!

Veeck
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Here is the new patch against kernel 2.6.5. Any comments?

Best regards
Michael Veeck

drivers/net/acenic.c +2 -0
@@ -335,9 +335,11 @@
 #define ACE_PROBE_ARG				struct net_device *dev
 #endif
 
+#if (LINUX_VERSION_CODE < 0x02040a)
 #ifndef min_t
 #define min_t(type,a,b)	(((a)<(b))?(a):(b))
 #endif
+#endif
 
 #ifndef ARCH_HAS_PREFETCHW
 #ifndef prefetchw

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#6 lore raw
In reply to Michael Veeck

On Wed, 07 Apr 2004 01:45:00 +0200 Michael Veeck wrote:

| Michael Veeck wrote:
| > 
| > Randy.Dunlap schrieb:
| > 
| >> On Sat, 07 Feb 2004 18:35:05 +0100 Michael Veeck 
| >> <[email protected]> wrote:
| >>
| >> | | | Jeff Garzik schrieb:
| >> | > Michael Veeck wrote:
| >> | > | >> Hi!
| >> | >> Patch (against 2.6.3-rc1) removes unnecessary min/max macros and 
| >> | >> changes calls to use kernel.h macros instead.
| >> | >> Feedback always welcome
| >> | >> Michael
| >> | >>
| >> | >>
| >> | >> 
| >> ------------------------------------------------------------------------
| >> | >>
| >> | >> --- linux-2.6.2.org/drivers/net/acenic.c    2004-02-07 | >> 
| >> 15:25:57.000000000 +0100
| >> | >> +++ linux-2.6.2.new/drivers/net/acenic.c    2004-02-07 | >> 
| >> 15:33:24.286033896 +0100
| >> | >> @@ -335,10 +335,6 @@
| >> | >>  #define ACE_PROBE_ARG                struct net_device *dev
| >> | >>  #endif
| >> | >>  | >> -#ifndef min_t
| >> | >> -#define min_t(type,a,b)    (((a)<(b))?(a):(b))
| >> | >> -#endif
| >> | > | > | > | > Read the code you're patching :)
| >> | > | > You're killing compat code the driver author added.
| >> | > | >     Jeff
| >> | > | | Ah, that didnt came into my mind. But since #ifndef min_t 
| >> doesnt seem to | be in any of the "#if LINUX_VERSION_CODE >= " why not 
| >> put it in the | appropiate one?
| >> | | Do you know when the min_t was introduced in include/linux/kernel.h ?
| >>
| >> BK shows 5-FEB-2002 for min_t.
| >> That would be early 2.5.x, like 2.5.4, estimating by looking at
| >> http://www.kernel.org/pub/linux/kernel/v2.5/
| >>
| > 
| > Digging through the changelogs on kernel.org I found out that 
| > min_t/max_t macros were really introduced in 2.4.9 as "min/max" and then 
| > changed to their min_t/max_t-name in 2.4.10.
| > 
| > I will incorproate that in my next patch! Thanks for the advice!
| > 
| 
| Here is the new patch against kernel 2.6.5. Any comments?

@@ -335,9 +335,11 @@
#define ACE_PROBE_ARG struct net_device *dev
#endif

+#if (LINUX_VERSION_CODE < 0x02040a)
#ifndef min_t
#define min_t(type,a,b) (((a)<(b))?(a):(b))
#endif
+#endif

#ifndef ARCH_HAS_PREFETCHW
#ifndef prefetchw

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When is "min_t" not defined? IOW, it looks redundant to me.

This local #define of min_t() evaluates 1 arg 2 times. Isnt' that a no-no? Wouldn't Jeff accept a patch for that?

--
~Randy
"We have met the enemy and he is us." -- Pogo (by Walt Kelly) (Again. Sometimes I think ln -s /usr/src/linux/.config .signature)

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors