RE: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
I don't see the reason this is needed so I'm going to say NAK.
Todd Fujinaka
Software Application Engineer
Networking Division (ND)
Intel Corporation
todd.fujin...@intel.com
(503) 712-4565
-Original Message-
From: Intel-wired-lan [mailto:intel-wired-lan-boun...@lists.osuosl.org] On
Behalf Of Maninder Singh
Sent: Monday, June 29, 2015 9:56 PM
To: Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn;
Skidmore, Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch A;
intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org;
linux-ker...@vger.kernel.org
Cc: Maninder Singh; panka...@samsung.com
Subject: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo
sizeof(a)/sizeof(a[0])
Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
Signed-off-by: Maninder Singh
Reviewed-by: Yogesh Narayan Gaur
---
drivers/net/ethernet/intel/igb/e1000_phy.c |6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_phy.c
b/drivers/net/ethernet/intel/igb/e1000_phy.c
index c1bb64d..e399b3c 100644
--- a/drivers/net/ethernet/intel/igb/e1000_phy.c
+++ b/drivers/net/ethernet/intel/igb/e1000_phy.c
@@ -37,8 +37,7 @@ static s32 igb_set_master_slave_mode(struct e1000_hw *hw);
static const u16 e1000_m88_cable_length_table[] = {
0, 50, 80, 110, 140, 140, E1000_CABLE_LENGTH_UNDEFINED }; #define
M88E1000_CABLE_LENGTH_TABLE_SIZE \
- (sizeof(e1000_m88_cable_length_table) / \
- sizeof(e1000_m88_cable_length_table[0]))
+ ARRAY_SIZE(e1000_m88_cable_length_table)
static const u16 e1000_igp_2_cable_length_table[] = {
0, 0, 0, 0, 0, 0, 0, 0, 3, 5, 8, 11, 13, 16, 18, 21, @@ -50,8 +49,7 @@
static const u16 e1000_igp_2_cable_length_table[] = {
83, 89, 95, 100, 105, 109, 113, 116, 119, 122, 124,
104, 109, 114, 118, 121, 124};
#define IGP02E1000_CABLE_LENGTH_TABLE_SIZE \
- (sizeof(e1000_igp_2_cable_length_table) / \
-sizeof(e1000_igp_2_cable_length_table[0]))
+ ARRAY_SIZE(e1000_igp_2_cable_length_table)
/**
* igb_check_reset_block - Check if PHY reset is blocked
--
1.7.9.5
___
Intel-wired-lan mailing list
intel-wired-...@lists.osuosl.org
http://lists.osuosl.org/mailman/listinfo/intel-wired-lan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
On Tue, Jun 30, 2015 at 4:53 PM, Fujinaka, Todd wrote: > I don't see the reason this is needed so I'm going to say NAK. Using generic functions is always better than open coded stuff. Linux's ARRAY_SIZE also makes sure that the passed variable is actually an array. -- Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
Sorry for the top-posting, but I'm provided with the tools they give me and bottom posting from Outlook just confuses email threads. Plus, this was crossposted all over creation and cc-ed to anyone with an intel address. I still would say no if I'm allowed, because to guarantee that this change - that I don't think fixes anything - works in all cases, we need to do an incredible amount of regression testing. Every variant of every Intel part that uses this driver (and there are many) should be tested and will end up being used by the community. Plus, you have no idea the number of obscure bugs I have to deal with as the guy answering customer questions. If this triggers some odd embedded compiler bug, I'm going to have to dig it out. Unless there is an actual bug, I'd like to leave it as it is. Todd Fujinaka Software Application Engineer Networking Division (ND) Intel Corporation todd.fujin...@intel.com (503) 712-4565 -Original Message- From: Richard Weinberger [mailto:richard.weinber...@gmail.com] Sent: Tuesday, June 30, 2015 12:01 PM To: Fujinaka, Todd Cc: Maninder Singh; Kirsher, Jeffrey T; Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore, Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-...@lists.osuosl.org; netdev@vger.kernel.org; linux-ker...@vger.kernel.org; panka...@samsung.com Subject: Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0]) On Tue, Jun 30, 2015 at 4:53 PM, Fujinaka, Todd wrote: > I don't see the reason this is needed so I'm going to say NAK. Using generic functions is always better than open coded stuff. Linux's ARRAY_SIZE also makes sure that the passed variable is actually an array. -- Thanks, //richard
Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
Hi! Am 30.06.2015 um 22:16 schrieb Fujinaka, Todd: > Sorry for the top-posting, but I'm provided with the tools they give me and > bottom posting from Outlook just confuses email threads. Plus, this was > crossposted all over creation and cc-ed to anyone with an intel address. > > I still would say no if I'm allowed, because to guarantee that this change - > that I don't think fixes anything - works in all cases, we need to do an > incredible amount of regression testing. Every variant of every Intel part > that uses this driver (and there are many) should be tested and will end up > being used by the community. > > Plus, you have no idea the number of obscure bugs I have to deal with as the > guy answering customer questions. If this triggers some odd embedded compiler > bug, I'm going to have to dig it out. Unless there is an actual bug, I'd like > to leave it as it is. If you don't dare to touch your driver please update it's maintenance status. "Supported" is definitely not the case, maybe "Odd fixes" would fit better. Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
On Tue, Jun 30, 2015 at 1:16 PM, Fujinaka, Todd wrote: > I still would say no if I'm allowed, because to guarantee that this change - > that I don't think fixes anything - works in all cases, we need to do an > incredible amount of regression testing. Every variant of every Intel part > that uses this driver (and there are many) should be tested and will end up > being used by the community. > Validation is really simple: diff old_module.ko new_module.ko And this is a good defensive measure, as it'll save you when someone screws up and changes your array to a pointer to an array (you'll get a build failure instead of 0). -- Alex Gartrell -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
On Tue, 2015-06-30 at 20:16 +, Fujinaka, Todd wrote: > Sorry for the top-posting, but I'm provided with the tools they give me > and bottom posting from Outlook just confuses email threads. Plus, this > was crossposted all over creation and cc-ed to anyone with an intel > address. Not quite. It was posted to the names listed under the MAINTAINERS entry. INTEL ETHERNET DRIVERS M: Jeff Kirsher R: Jesse Brandeburg R: Shannon Nelson R: Carolyn Wyborny R: Don Skidmore R: Matthew Vick R: John Ronciak R: Mitch Williams L: intel-wired-...@lists.osuosl.org btw: You aren't listed there Todd. Should you be? > I still would say no if I'm allowed, because to guarantee that this > change - that I don't think fixes anything Simplicity for the reader is generally a good thing. Removing the macros altogether is likely better. > - works in all cases, we > need to do an incredible amount of regression testing. Compilers should not produce different object code. Verification of no object changes should be good enough. > Every variant of > every Intel part that uses this driver (and there are many) should be > tested and will end up being used by the community. > > Plus, you have no idea the number of obscure bugs I have to deal with > as the guy answering customer questions. If this triggers some odd > embedded compiler bug, I'm going to have to dig it out. Unless there is > an actual bug, I'd like to leave it as it is. If any compiler miscompiles the ARRAY_SIZE macro, there are bound to be real issues with using that compiler in a production environment. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
I've submitted a version to intel-wired-lan that addresses the original issue as well as Joe Perches's suggestion to just delete the define. Todd Fujinaka Software Application Engineer Networking Division (ND) Intel Corporation todd.fujin...@intel.com (503) 712-4565 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH 1/1] igb: Use ARRAY_SIZE instead fo sizeof(a)/sizeof(a[0])
On Tue, 2015-06-30 at 22:25 +, Fujinaka, Todd wrote: > I've submitted a version to intel-wired-lan that addresses the original > issue as well as Joe Perches's suggestion to just delete the define. Please cc netdev next time too. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
