| Message ID | 1406766731-4209-7-git-send-email-aaron.f.brown@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Aaron Brown <aaron.f.brown@intel.com> Date: Wed, 30 Jul 2014 17:32:02 -0700 > From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > > Declare some variables once outside of the loop instead of in > every iteration of the loop. > > Change-ID: I436913777c7da3c16dc0031b59e3ffa61de74718 > Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > Tested-by: Jim Young <jamesx.m.young@intel.com> > Signed-off-by: Aaron Brown <aaron.f.brown@intel.com> Why? Decreasing the scope of variables makes the code easier to understand and audit. Now that it's declared at the top level people have to check whether is it initialized properly in all possible code paths, in the entire function, rather than just inside of the loop. Please respin this series with this patch removed, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Thursday, July 31, 2014 2:06 PM > To: Brown, Aaron F > Cc: Abodunrin, Akeem G; netdev@vger.kernel.org; gospo@redhat.com; > sassmann@redhat.com > Subject: Re: [net-next 06/15] i40e: Move some variable declarations out of a > loop > > From: Aaron Brown <aaron.f.brown@intel.com> > Date: Wed, 30 Jul 2014 17:32:02 -0700 > > > From: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > > > > Declare some variables once outside of the loop instead of in every > > iteration of the loop. > > > > Change-ID: I436913777c7da3c16dc0031b59e3ffa61de74718 > > Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com> > > Tested-by: Jim Young <jamesx.m.young@intel.com> > > Signed-off-by: Aaron Brown <aaron.f.brown@intel.com> > > Why? Decreasing the scope of variables makes the code easier to understand > and audit. > > Now that it's declared at the top level people have to check whether is it > initialized properly in all possible code paths, in the entire function, rather than > just inside of the loop. > > Please respin this series with this patch removed, thanks. This is done because other codes have to use it before the loop... Hope you understand the reason for this - and that is how it supposed to be originally, we have to increase the scope of the variables so that other code that are not in the loop can use it. Regards, ~Akeem G Abodunrin -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Abodunrin, Akeem G" <akeem.g.abodunrin@intel.com> Date: Thu, 31 Jul 2014 23:56:19 +0000 > This is done because other codes have to use it before the loop... > > Hope you understand the reason for this - and that is how it supposed to be originally, we have to increase the scope of the variables so that other code that are not in the loop can use it. What codes? I looked at the patches after this one and none of them use the variables. If anything you should make this change in the patch that needs it, not beforehand. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 6ac8487..66a1070 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -812,7 +812,10 @@ static void i40e_update_vsi_stats(struct i40e_vsi *vsi) struct i40e_eth_stats *oes; struct i40e_eth_stats *es; /* device's eth stats */ u32 tx_restart, tx_busy; + struct i40e_ring *p; u32 rx_page, rx_buf; + u64 bytes, packets; + unsigned int start; u64 rx_p, rx_b; u64 tx_p, tx_b; u16 q; @@ -836,10 +839,6 @@ static void i40e_update_vsi_stats(struct i40e_vsi *vsi) rx_buf = 0; rcu_read_lock(); for (q = 0; q < vsi->num_queue_pairs; q++) { - struct i40e_ring *p; - u64 bytes, packets; - unsigned int start; - /* locate Tx ring */ p = ACCESS_ONCE(vsi->tx_rings[q]);