On 11/6/25 17:05, ally heev wrote:
On Thu, 2025-11-06 at 15:07 +0100, Alexander Lobakin wrote:
[..]
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c
b/drivers/net/ethernet/intel/ice/ice_flow.c
index
6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa
100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16
dest_vsi, u16 fdir_vsi,
struct ice_parser_profile *prof, enum ice_block blk)
{
u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
- struct ice_flow_prof_params *params __free(kfree);
u8 fv_words = hw->blk[blk].es.fvw;
int status;
int i, idx;
- params = kzalloc(sizeof(*params), GFP_KERNEL);
+ struct ice_flow_prof_params *params __free(kfree) =
+ kzalloc(sizeof(*params), GFP_KERNEL);
Please don't do it that way. It's not C++ with RAII and
declare-where-you-use.
Just leave the variable declarations where they are, but initialize them
with `= NULL`.
+1
Variable declarations must be in one block and sorted from the longest
to the shortest.
But most important, I'm not even sure how you could trigger an
"undefined behaviour" here. Both here and below the variable tagged with
`__free` is initialized right after the declaration block, before any
return. So how to trigger an UB here?
It doesn't occur here. But, many maintainers/developers consider it a
bad practice because if the function returns before initialization or
use of `goto` can cause such behaviors.
we were bitten by that already, scenario is as follow:
0. have a good code w/o UB and w/o redundant = NULL
1. add some early return, say:
if (dest_vsi == fdir_vsi)
return -EINVAL;
2. almost granted that person adding 1. will forget to add = NULL to all
declarations marked __free
Here though, the definitions are still at the top right? Maybe I could
just sort them
we discourage putting any operations, including allocations, that may
fail into the declarations block
