From: Julia Lawall <julia@diku.dk>

The use of & to obtain a conjunction that evaluates both of its arguments
seems unnecessarily tricky.

The semantic match that found this code is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@ expression E1, E2; @@
*  !E1 & !E2
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/e1000/e1000_main.c |   16 ++++++++++------
 drivers/net/ixgb/ixgb_main.c   |   11 ++++++-----
 2 files changed, 16 insertions(+), 11 deletions(-)

diff -u -p a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
--- a/drivers/net/e1000/e1000_main.c	2008-02-20 22:09:26.000000000 +0100
+++ b/drivers/net/e1000/e1000_main.c	2008-03-06 17:30:20.000000000 +0100
@@ -3872,10 +3872,12 @@ e1000_intr_msi(int irq, void *data)
 	adapter->total_tx_packets = 0;
 	adapter->total_rx_packets = 0;

-	for (i = 0; i < E1000_MAX_INTR; i++)
-		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
-		   !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
+	for (i = 0; i < E1000_MAX_INTR; i++) {
+		boolean_t c1 = adapter->clean_rx(adapter, adapter->rx_ring);
+		boolean_t c2 = e1000_clean_tx_irq(adapter, adapter->tx_ring);
+		if (unlikely(!c1 && !c2))
 			break;
+	}

 	if (likely(adapter->itr_setting & 3))
 		e1000_set_itr(adapter);
@@ -3974,10 +3976,12 @@ e1000_intr(int irq, void *data)
 	adapter->total_tx_packets = 0;
 	adapter->total_rx_packets = 0;

-	for (i = 0; i < E1000_MAX_INTR; i++)
-		if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
-		   !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
+	for (i = 0; i < E1000_MAX_INTR; i++) {
+		boolean_t c1 = adapter->clean_rx(adapter, adapter->rx_ring);
+		boolean_t c2 = e1000_clean_tx_irq(adapter, adapter->tx_ring);
+		if (unlikely(!c1 && !c2))
 			break;
+	}

 	if (likely(adapter->itr_setting & 3))
 		e1000_set_itr(adapter);
diff -u...
[ message continues ]

Perhaps also a s/boolean_t/bool/g kernel wide?

$ grep -wrl boolean_t *

drivers/net/e1000/e1000.h
drivers/net/e1000/e1000_ethtool.c
drivers/net/e1000/e1000_hw.c
drivers/net/e1000/e1000_hw.h
drivers/net/e1000/e1000_main.c
drivers/net/e1000/e1000_osdep.h
drivers/net/ixgb/ixgb.h
drivers/net/ixgb/ixgb_ee.c
drivers/net/ixgb/ixgb_ee.h
drivers/net/ixgb/ixgb_ethtool.c
drivers/net/ixgb/ixgb_hw.c
drivers/net/ixgb/ixgb_hw.h
drivers/net/ixgb/ixgb_main.c
drivers/net/ixgb/ixgb_osdep.h
fs/xfs/quota/xfs_dquot.c
fs/xfs/quota/xfs_qm.c
fs/xfs/quota/xfs_qm.h
fs/xfs/quota/xfs_qm_syscalls.c
fs/xfs/quota/xfs_trans_dquot.c
fs/xfs/xfs_ialloc.c
fs/xfs/xfs_ialloc.h
fs/xfs/xfs_inode.c
fs/xfs/xfs_inode.h
fs/xfs/xfs_log.c
fs/xfs/xfs_vfsops.c
fs/xfs/xfs_utils.c
fs/xfs/xfs_types.h
fs/xfs/xfs_vnodeops.c


--

send me a patch for e1000 and for ixgb and I'll happily apply those :)

(which, BTW also could use the uint32_t -> u32 (etc) changes... while you're at it)


--

I think this does what you want:

for size in "8" "16" "32" "64" ; do \
sed -r -i -e 's/\bu_{0,1}int'$size'_t\b/u'$size'/g' \
$(grep -rPlw --include=*.[ch] 'u_{0,1}int'$size'_t' drivers/net/e1000 drivers/net/ixgb); done

But why?  boolean_t is used by 3 subsystems with local typedefs.
These others are much more frequently used by kernel source.

$ grep -rPlw --include=*.[ch] "u{0,1}_{0,1}int(8|16|32|64)_t" * | wc -l
876

include/linux/types.h has typedefs for these but not boolean_t

include/linux/types.h:typedef           __u8            u_int8_t;
include/linux/types.h:typedef           __s8            int8_t;
include/linux/types.h:typedef           __u16           u_int16_t;
include/linux/types.h:typedef           __s16           int16_t;
include/linux/types.h:typedef           __u32           u_int32_t;
include/linux/types.h:typedef           __s32           int32_t;
include/linux/types.h:typedef           __u8            uint8_t;
include/linux/types.h:typedef           __u16           uint16_t;
include/linux/types.h:typedef           __u32           uint32_t;
include/linux/types.h:typedef           __u64           uint64_t;
include/linux/types.h:typedef           __u64           u_int64_t;
include/linux/types.h:typedef           __s64           int64_t;

cheers, Joe

--

Why not the other way around ?
inttypes.h is C99 and defines int16_t (or uint16_t). i do not see any reason
not to use them or any other type (like __u8) that does actually the same.

In that case we do not need different type names for userspace/kernelspace, or is there an other name
for int ? in that case we can remove the whole heap of typedef's.

re,
 wh



--

afaik they're really meant for userspace related code and don't belong in our

yes, a lot of that *is* userspace related code.

do the same search in drivers/net/ .... you'll see the only drivers using this for
everything is our old drivers...

Auke
--

boolean_t to bool
TRUE to true
FALSE to false
comment typo ahread to ahead

Signed-off-by: Joe Perches <joe@perches.com>

 drivers/net/e1000/e1000.h         |   26 ++--
 drivers/net/e1000/e1000_ethtool.c |   17 ++--
 drivers/net/e1000/e1000_hw.c      |  222 ++++++++++++++++++-------------------
 drivers/net/e1000/e1000_hw.h      |   62 +++++-----
 drivers/net/e1000/e1000_main.c    |   90 ++++++++--------
 drivers/net/e1000/e1000_osdep.h   |    7 -
 6 files changed, 208 insertions(+), 216 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 3b84028..d73a6c1 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -188,7 +188,7 @@ struct e1000_tx_ring {
 	spinlock_t tx_lock;
 	uint16_t tdh;
 	uint16_t tdt;
-	boolean_t last_tx_tso;
+	bool last_tx_tso;
 };
 
 struct e1000_rx_ring {
@@ -283,17 +283,17 @@ struct e1000_adapter {
 	uint32_t tx_fifo_size;
 	uint8_t  tx_timeout_factor;
 	atomic_t tx_fifo_stall;
-	boolean_t pcix_82544;
-	boolean_t detect_tx_hung;
+	bool pcix_82544;
+	bool detect_tx_hung;
 
 	/* RX */
 #ifdef CONFIG_E1000_NAPI
-	boolean_t (*clean_rx) (struct e1000_adapter *adapter,
-			       struct e1000_rx_ring *rx_ring,
-			       int *work_done, int work_to_do);
+	bool (*clean_rx) (struct e1000_adapter *adapter,
+			  struct e1000_rx_ring *rx_ring,
+			  int *work_done, int work_to_do);
 #else
-	boolean_t (*clean_rx) (struct e1000_adapter *adapter,
-			       struct e1000_rx_ring *rx_ring);
+	bool (*clean_rx) (struct e1000_adapter *adapter,
+			  struct e1000_rx_ring *rx_ring);
 #endif
 	void (*alloc_rx_buf) (struct e1000_adapter *adapter,
 			      struct e1000_rx_ring *rx_ring,
@@ -312,7 +312,7 @@ struct e1000_adapter {
 	uint32_t alloc_rx_buff_failed;
 	uint32_t rx_int_delay;
 	uint32_t rx_abs_int_delay;
-	boolean_t rx_csum;
+	bool rx_csum;
 	unsigned int rx_ps_pages;
 	uint32_t gorcl;
 	uint64_t gorcl_old;
@@ -335,12 +335,12 @@ struct e1000_adapter {
 	struct e1000_rx_ring test_rx...
[ message continues ]

> send me a patch for e1000 and for ixgb and I'll happily apply those :)

boolean_t to bool
TRUE to true
FALSE to false

Signed-off-by: Joe Perches <joe@perches.com>

 drivers/net/ixgb/ixgb.h         |    8 +++---
 drivers/net/ixgb/ixgb_ee.c      |   50 +++++++++++++++++-----------------
 drivers/net/ixgb/ixgb_ee.h      |    2 +-
 drivers/net/ixgb/ixgb_ethtool.c |   10 +++---
 drivers/net/ixgb/ixgb_hw.c      |   56 +++++++++++++++++++-------------------
 drivers/net/ixgb/ixgb_hw.h      |   18 ++++++------
 drivers/net/ixgb/ixgb_main.c    |   44 +++++++++++++++---------------
 drivers/net/ixgb/ixgb_osdep.h   |    7 -----
 8 files changed, 94 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ixgb/ixgb.h b/drivers/net/ixgb/ixgb.h
index 3d2e721..2066161 100644
--- a/drivers/net/ixgb/ixgb.h
+++ b/drivers/net/ixgb/ixgb.h
@@ -173,15 +173,15 @@ struct ixgb_adapter {
 	uint64_t hw_csum_tx_error;
 	uint32_t tx_int_delay;
 	uint32_t tx_timeout_count;
-	boolean_t tx_int_delay_enable;
-	boolean_t detect_tx_hung;
+	bool tx_int_delay_enable;
+	bool detect_tx_hung;
 
 	/* RX */
 	struct ixgb_desc_ring rx_ring;
 	uint64_t hw_csum_rx_error;
 	uint64_t hw_csum_rx_good;
 	uint32_t rx_int_delay;
-	boolean_t rx_csum;
+	bool rx_csum;
 
 	/* OS defined structs */
 	struct napi_struct napi;
@@ -194,7 +194,7 @@ struct ixgb_adapter {
 	u16 msg_enable;
 	struct ixgb_hw_stats stats;
 	uint32_t alloc_rx_buff_failed;
-	boolean_t have_msi;
+	bool have_msi;
 };
 
 /* Exported from other modules */
diff --git a/drivers/net/ixgb/ixgb_ee.c b/drivers/net/ixgb/ixgb_ee.c
index e8eb0fd..1c57ded 100644
--- a/drivers/net/ixgb/ixgb_ee.c
+++ b/drivers/net/ixgb/ixgb_ee.c
@@ -36,7 +36,7 @@ static void ixgb_shift_out_bits(struct ixgb_hw *hw,
 				uint16_t count);
 static void ixgb_standby_eeprom(struct ixgb_hw *hw);
 
-static boolean_t ixgb_wait_eeprom_command(struct ixgb_hw *hw);
+static bool ixgb_wait_eeprom_command(struct ixgb_hw *hw);
 
 static void ixgb_cleanup_eeprom(struct ix...
[ message continues ]

thanks Joe, I'll apply both. (I'll fix up the checkpatch warnings)


--