Re: [Kernel-janitors] [PATCH] drivers/net/acenic.c MIN/MAX removal

["Randy.Dunlap" <rddunlap@osdl.org>]

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

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 
| >> <michael.veeck@gmx.net> 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)

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors