| Message ID | 20241204150224.346606-1-mateusz.polchlopek@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [iwl-net,v1] ice: move static_assert to declaration section | expand |
Dear Mateusz, Thank you for the patch. Am 04.12.24 um 16:02 schrieb Mateusz Polchlopek: > static_assert() needs to be placed in the declaration section, > so move it there in ice_cfg_tx_topo() function. > > Current code causes following warnings on some gcc versions: Please list the versions you know of. > error: ISO C90 forbids mixed declarations and code > [-Werror=declaration-after-statement] The above could be in one line, as it’s pasted. > Fixes: c188afdc3611 ("ice: fix memleak in ice_init_tx_topology()") > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com> > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_ddp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c > index 69d5b1a28491..e885f84520ba 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ddp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c > @@ -2388,6 +2388,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len) > int status; > u8 flags; > > + static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); > + > if (!buf || !len) > return -EINVAL; > > @@ -2482,7 +2484,6 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len) > } > > /* Get the new topology buffer, reuse current topo copy mem */ > - static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); > new_topo = topo; > memcpy(new_topo, (u8 *)section + offset, size); The diff looks good. Kind regards, Paul
On 12/4/2024 4:05 PM, Paul Menzel wrote: > Dear Mateusz, > > > Thank you for the patch. > > Am 04.12.24 um 16:02 schrieb Mateusz Polchlopek: >> static_assert() needs to be placed in the declaration section, >> so move it there in ice_cfg_tx_topo() function. >> >> Current code causes following warnings on some gcc versions: > > Please list the versions you know of. > Sure, in next version I will add the info. >> error: ISO C90 forbids mixed declarations and code >> [-Werror=declaration-after-statement] > > The above could be in one line, as it’s pasted. > Okay, it will be fixed in v2 >> Fixes: c188afdc3611 ("ice: fix memleak in ice_init_tx_topology()") >> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com> >> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >> --- >> drivers/net/ethernet/intel/ice/ice_ddp.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ >> ethernet/intel/ice/ice_ddp.c >> index 69d5b1a28491..e885f84520ba 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c >> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c >> @@ -2388,6 +2388,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const >> void *buf, u32 len) >> int status; >> u8 flags; >> + static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); >> + >> if (!buf || !len) >> return -EINVAL; >> @@ -2482,7 +2484,6 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const >> void *buf, u32 len) >> } >> /* Get the new topology buffer, reuse current topo copy mem */ >> - static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); >> new_topo = topo; >> memcpy(new_topo, (u8 *)section + offset, size); > > The diff looks good. > > > Kind regards, > > Paul Thanks Paul for review! BR
On 12/5/2024 2:18 PM, Mateusz Polchlopek wrote: > > > On 12/4/2024 4:05 PM, Paul Menzel wrote: >> Dear Mateusz, >> >> >> Thank you for the patch. >> >> Am 04.12.24 um 16:02 schrieb Mateusz Polchlopek: >>> static_assert() needs to be placed in the declaration section, >>> so move it there in ice_cfg_tx_topo() function. >>> >>> Current code causes following warnings on some gcc versions: >> >> Please list the versions you know of. >> > > Sure, in next version I will add the info. > >>> error: ISO C90 forbids mixed declarations and code >>> [-Werror=declaration-after-statement] >> >> The above could be in one line, as it’s pasted. >> > > Okay, it will be fixed in v2 > >>> Fixes: c188afdc3611 ("ice: fix memleak in ice_init_tx_topology()") >>> Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com> >>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> >>> --- >>> drivers/net/ethernet/intel/ice/ice_ddp.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ >>> ethernet/intel/ice/ice_ddp.c >>> index 69d5b1a28491..e885f84520ba 100644 >>> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c >>> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c >>> @@ -2388,6 +2388,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const >>> void *buf, u32 len) >>> int status; >>> u8 flags; >>> + static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); >>> + >>> if (!buf || !len) >>> return -EINVAL; >>> @@ -2482,7 +2484,6 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const >>> void *buf, u32 len) >>> } >>> /* Get the new topology buffer, reuse current topo copy mem */ >>> - static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); >>> new_topo = topo; >>> memcpy(new_topo, (u8 *)section + offset, size); >> >> The diff looks good. >> >> >> Kind regards, >> >> Paul > > Thanks Paul for review! > > BR > > Ach... It occurred that this is not the problem with compiler version but with re-introduction of the -Wdeclaration-after-statement flag in my test setup :/ Nevertheless this move of static_assert is still considered as a good practice, so I will send v2 to the net-next with some other polishing of code as an improvement (and I will drop Fixes: tag).
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c index 69d5b1a28491..e885f84520ba 100644 --- a/drivers/net/ethernet/intel/ice/ice_ddp.c +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c @@ -2388,6 +2388,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len) int status; u8 flags; + static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); + if (!buf || !len) return -EINVAL; @@ -2482,7 +2484,6 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len) } /* Get the new topology buffer, reuse current topo copy mem */ - static_assert(ICE_PKG_BUF_SIZE == ICE_AQ_MAX_BUF_LEN); new_topo = topo; memcpy(new_topo, (u8 *)section + offset, size);
static_assert() needs to be placed in the declaration section, so move it there in ice_cfg_tx_topo() function. Current code causes following warnings on some gcc versions: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] Fixes: c188afdc3611 ("ice: fix memleak in ice_init_tx_topology()") Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> --- drivers/net/ethernet/intel/ice/ice_ddp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)