Skip to contentSkip to conversation

drivers/net/wireless/strip.c MIN/MAX removal

PATCH In review Michael Veeck 2 participants 4 messages Feb 7, 2004

Subsystems (2)
Kernel janitors Networking drivers (wireless)
lore
Share & tools
lore

Michael Veeck

author
#0 lore raw

PATCH 1/1 drivers/net/wireless/strip.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/wireless/strip.c +3 -5
diff -Naur linux-2.6.2.org/drivers/net/wireless/strip.c linux-2.6.2.new/drivers/net/wireless/strip.c
@@ -88,7 +88,7 @@
 #include <asm/uaccess.h>
 #include <asm/bitops.h>
 
-# include <linux/ctype.h>
+#include <linux/ctype.h>
 #include <linux/string.h>
 #include <linux/mm.h>
 #include <linux/interrupt.h>

@@ -454,8 +454,6 @@
 
 #define READDEC(X) ((X)>='0' && (X)<='9' ? (X)-'0' : 0)
 
-#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
-#define MAX(X, Y) ((X) > (Y) ? (X) : (Y))
 #define ELEMENTS_OF(X) (sizeof(X) / sizeof((X)[0]))
 #define ARRAY_END(X) (&((X)[ELEMENTS_OF(X)]))
 

@@ -847,7 +845,7 @@
 static int allocate_buffers(struct strip *strip_info, int mtu)
 {
 	struct net_device *dev = strip_info->dev;
-	int sx_size = MAX(STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
+	int sx_size = max((int)STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
 	int tx_size = STRIP_ENCAP_SIZE(mtu) + MaxCommandStringLength;
 	__u8 *r = kmalloc(MAX_RECV_MTU, GFP_ATOMIC);
 	__u8 *s = kmalloc(sx_size, GFP_ATOMIC);

@@ -1709,7 +1707,7 @@
 	p++;
 
 	len = value_end - value_begin;
-	len = MIN(len, sizeof(FirmwareVersion) - 1);
+	len = min(len, (int) sizeof(FirmwareVersion) - 1);
 	if (strip_info->firmware_version.c[0] == 0)
 		printk(KERN_INFO "%s: Radio Firmware: %.*s\n",
 		       strip_info->dev->name, len, value_begin);

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

#1 lore raw

Re: [Kernel-janitors] [PATCH] drivers/net/wireless/strip.c MIN/MAX

Quoted 5 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

Just some suggestions...

drivers/net/wireless/strip.c line 457

Quoted 7 lines
drivers/net/wireless/strip.c +0 -2
@@ -454,8 +454,6 @@
 
 #define READDEC(X) ((X)>='0' && (X)<='9' ? (X)-'0' : 0)
 
-#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
-#define MAX(X, Y) ((X) > (Y) ? (X) : (Y))
 #define ELEMENTS_OF(X) (sizeof(X) / sizeof((X)[0]))

Remove this define and s/ELEMENTS_OF/ARRAY_SIZE/g

drivers/net/wireless/strip.c line 848

Quoted 6 lines
drivers/net/wireless/strip.c +1 -1
@@ -847,7 +845,7 @@
 static int allocate_buffers(struct strip *strip_info, int mtu)
 {
        struct net_device *dev = strip_info->dev;
-       int sx_size = MAX(STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
+       int sx_size = max((int)STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);

max_t?

Domen

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

author
#2 lore raw

Re: [Kernel-janitors] [PATCH] drivers/net/wireless/strip.c MIN/MAX

In reply to Michael Veeck
Quoted 7 lines · Domen Puncer

Just some suggestions...

Quoted 1 line

#define ELEMENTS_OF(X) (sizeof(X) / sizeof((X)[0]))

Remove this define and s/ELEMENTS_OF/ARRAY_SIZE/g

There are more occurances of redundant ARRAY_SIZEs in the kernel. I will keep that in mind!

Quoted 11 lines

Review line 848

Quoted 8 lines
(quoted diff) +1 -1
@@ -847,7 +845,7 @@
 static int allocate_buffers(struct strip *strip_info, int mtu)
 {
       struct net_device *dev = strip_info->dev;
-       int sx_size = MAX(STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
+       int sx_size = max((int)STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);

max_t?

Changed that to max_t and min_t. A newbie queston: What should when be preferred and why?

Attached is the new patch.

Veeck

drivers/net/wireless/strip.c +5 -7
@@ -82,6 +82,7 @@
 /* Header files								*/
 
 #include <linux/config.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <asm/system.h>

@@ -454,10 +455,7 @@
 
 #define READDEC(X) ((X)>='0' && (X)<='9' ? (X)-'0' : 0)
 
-#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
-#define MAX(X, Y) ((X) > (Y) ? (X) : (Y))
-#define ELEMENTS_OF(X) (sizeof(X) / sizeof((X)[0]))
-#define ARRAY_END(X) (&((X)[ELEMENTS_OF(X)]))
+#define ARRAY_END(X) (&((X)[ARRAY_SIZE(X)]))
 
 #define JIFFIE_TO_SEC(X) ((X) / HZ)
 

@@ -847,7 +845,7 @@
 static int allocate_buffers(struct strip *strip_info, int mtu)
 {
 	struct net_device *dev = strip_info->dev;
-	int sx_size = MAX(STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
+	int sx_size = max_t(int, STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
 	int tx_size = STRIP_ENCAP_SIZE(mtu) + MaxCommandStringLength;
 	__u8 *r = kmalloc(MAX_RECV_MTU, GFP_ATOMIC);
 	__u8 *s = kmalloc(sx_size, GFP_ATOMIC);

@@ -1465,7 +1463,7 @@
 		/* Cycle to next periodic command? */
 		if (strip_info->firmware_level >= StructuredMessages)
 			if (++strip_info->next_command >=
-			    ELEMENTS_OF(CommandString))
+			    ARRAY_SIZE(CommandString))
 				strip_info->next_command = 0;
 #ifdef EXT_COUNTERS
 		strip_info->tx_ebytes += ts.length;

@@ -1709,7 +1707,7 @@
 	p++;
 
 	len = value_end - value_begin;
-	len = MIN(len, sizeof(FirmwareVersion) - 1);
+	len = min_t(int, len, sizeof(FirmwareVersion) - 1);
 	if (strip_info->firmware_version.c[0] == 0)
 		printk(KERN_INFO "%s: Radio Firmware: %.*s\n",
 		       strip_info->dev->name, len, value_begin);

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

#3 lore raw

Re: [Kernel-janitors] [PATCH] drivers/net/wireless/strip.c MIN/MAX

In reply to Michael Veeck

On Saturday 07 of February 2004 21:24, Michael Veeck wrote:

<snip>

Quoted 7 lines
Quoted 4 lines

drivers/net/wireless/strip.c line 848

Quoted 2 lines
drivers/net/wireless/strip.c +1 -1
-       int sx_size = MAX(STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);
+       int sx_size = max((int)STRIP_ENCAP_SIZE(MAX_RECV_MTU), 4096);

max_t?

Changed that to max_t and min_t. A newbie queston: What should when be preferred and why?

min_t/max_t when there are different type arguments. Because it looks nicer? :-)

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