Sun, Feb 10, 2019 at 07:28:46PM CET, a...@mellanox.com wrote:
>Introducing argument's metadata structure matching a bitmap flag per
>required argument and an error message if missing. Using this static
>array to refactor validation of finding required arguments in devlink
>command line and to ease further maintenance.
>
>Signed-off-by: Aya Levin <a...@mellanox.com>
>Reviewed-by: Moshe Shemesh <mo...@mellanox.com>
>---
> devlink/devlink.c | 155 +++++++++++++++++-------------------------------------
> 1 file changed, 47 insertions(+), 108 deletions(-)
>
>diff --git a/devlink/devlink.c b/devlink/devlink.c
>index d823512a4030..a05755385a49 100644
>--- a/devlink/devlink.c
>+++ b/devlink/devlink.c
>@@ -39,6 +39,7 @@
> #define PARAM_CMODE_RUNTIME_STR "runtime"
> #define PARAM_CMODE_DRIVERINIT_STR "driverinit"
> #define PARAM_CMODE_PERMANENT_STR "permanent"
>+#define DL_ARGS_REQUIRED_MAX_ERR_LEN 80
>
> static int g_new_line_count;
>
>@@ -950,6 +951,51 @@ static int param_cmode_get(const char *cmodestr,
> return 0;
> }
>
>+struct dl_args_metadata {
>+ uint32_t o_flag;
>+ char err_msg[DL_ARGS_REQUIRED_MAX_ERR_LEN];
>+};
>+
>+static const struct dl_args_metadata dl_args_required[] = {
>+ {DL_OPT_PORT_TYPE, "Port type not set.\n"},
>+ {DL_OPT_PORT_COUNT, "Port split count option expected.\n"},
>+ {DL_OPT_SB_POOL, "Pool index option expected.\n"},
>+ {DL_OPT_SB_SIZE, "Pool size option expected.\n"},
>+ {DL_OPT_SB_TYPE, "Pool type option expected.\n"},
>+ {DL_OPT_SB_THTYPE, "Pool threshold type option expected.\n"},
>+ {DL_OPT_SB_TH, "Threshold option expected.\n"},
>+ {DL_OPT_SB_TC, "TC index option expected.\n"},
>+ {DL_OPT_ESWITCH_MODE, "E-Switch mode option expected.\n"},
>+ {DL_OPT_ESWITCH_INLINE_MODE, "E-Switch inline-mode option
>expected.\n"},
>+ {DL_OPT_DPIPE_TABLE_NAME, "Dpipe table name expected\n"},
>+ {DL_OPT_DPIPE_TABLE_COUNTERS, "Dpipe table counter state expected\n"},
>+ {DL_OPT_ESWITCH_ENCAP_MODE, "E-Switch encapsulation option
>expected.\n"},
>+ {DL_OPT_PARAM_NAME, "Parameter name expected.\n"},
>+ {DL_OPT_PARAM_VALUE, "Value to set expected.\n"},
>+ {DL_OPT_PARAM_CMODE, "Configuration mode expected.\n"},
>+ {DL_OPT_REGION_SNAPSHOT_ID, "Region snapshot id expected.\n"},
>+ {DL_OPT_REGION_ADDRESS, "Region address value expected.\n"},
>+ {DL_OPT_REGION_LENGTH, "Region length value expected.\n"},
Remove the "\n" from there and put it to the pr_err() call.
>+};
>+
>+static int validate_finding_required_dl_args(uint32_t o_required,
Please maintain the current naming scheme of functions. This should be
named something like:
dl_args_finding_required_validate()
>+ uint32_t o_found)
>+{
>+ uint32_t dl_args_required_size;
>+ uint32_t o_flag;
>+ int i;
>+
>+ dl_args_required_size = ARRAY_SIZE(dl_args_required);
>+ for (i = 0; i < dl_args_required_size; i++) {
>+ o_flag = dl_args_required[i].o_flag;
>+ if ((o_required & o_flag) && !(o_found & o_flag)) {
>+ pr_err("%s", dl_args_required[i].err_msg);
>+ return -EINVAL;
>+ }
>+ }
>+ return 0;
>+}
>+
[...]
