On 13-Sep-24 11:08, Dan Carpenter wrote:
Hello Yevgeny Kliteynik,
Commit 472dd792348f ("net/mlx5: HWS, added matchers functionality")
from Jun 20, 2024 (linux-next), leads to the following Smatch static
checker warnings:
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_matcher.c:970 mlx5hws_matcher_attach_at() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_table.c:492 mlx5hws_table_set_default_miss() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:754 mlx5hws_rule_destroy() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:758 mlx5hws_rule_destroy() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:770 mlx5hws_rule_action_update() warn: should this return really be negated?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_rule.c:779 mlx5hws_rule_action_update() warn: should this return really be negated?
All of the above was fixed by the later patch that has already been accepted:
3f4c38df5b0f net/mlx5: HWS, fixed error flow return values of some functions
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_bwc_complex.c:36 mlx5hws_bwc_match_params_is_complex() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_definer.c:1848 hws_definer_find_best_match_fit() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_definer.c:1934 mlx5hws_definer_calc_layout() warn: was negative '-E2BIG' intended?
drivers/net/ethernet/mellanox/mlx5/core/steering/hws/mlx5hws_matcher.c:678 hws_matcher_bind_mt() warn: was negative '-E2BIG' intended?
The "E2BIG" was intentional.
However, I do agree with you that mixing positive and negative error codes is a bad idea.
In terms of future proofing the code, I think mixing positive and negative error
codes is risky. Eventually someone will write an error check like if (ret < 0)
and it will be treated as success or someone will pass the positive value to
ERR_PTR() and it will cause an error pointer dereference.
Agree, will send a fix later to make all the return codes negative.
Thanks!
-- YK
971
972 required_stes = at->num_of_action_stes - (!is_jumbo || at->only_term);
973 if (matcher->action_ste[MLX5HWS_ACTION_STE_IDX_ANY].max_stes < required_stes) {
974 mlx5hws_dbg(ctx, "Required STEs [%d] exceeds initial action template STE [%d]\n",
975 required_stes,
976 matcher->action_ste[MLX5HWS_ACTION_STE_IDX_ANY].max_stes);
977 return -ENOMEM;
978 }
979
980 matcher->at[matcher->num_of_at] = *at;
981 matcher->num_of_at += 1;
982 matcher->attr.max_num_of_at_attach -= 1;
983
984 if (matcher->col_matcher)
985 matcher->col_matcher->num_of_at = matcher->num_of_at;
986
987 return 0;
988 }
regards,
dan carpenter
