| Message ID | 20240218194413.31742-1-maks.mishinFZ@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | m_ife: Remove unused value | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Sun, 18 Feb 2024 22:44:13 +0300 Maks Mishin <maks.mishinfz@gmail.com> wrote: > The variable `has_optional` do not used after set the value. > Found by RASU JSC. > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> Yes, it is set after used. The flag should be removed all together and fold the newline into previous clause? And since the if clauses are now single statement, the code style from checkpatch wants braces to be removed. Something like this: diff --git a/tc/m_ife.c b/tc/m_ife.c index 162607ce7415..fce5af784e39 100644 --- a/tc/m_ife.c +++ b/tc/m_ife.c @@ -219,7 +219,6 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) __u32 mmark = 0; __u16 mtcindex = 0; __u32 mprio = 0; - int has_optional = 0; SPRINT_BUF(b2); print_string(PRINT_ANY, "kind", "%s ", "ife"); @@ -240,12 +239,9 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) if (tb[TCA_IFE_TYPE]) { ife_type = rta_getattr_u16(tb[TCA_IFE_TYPE]); - has_optional = 1; print_0xhex(PRINT_ANY, "type", "type %#llX ", ife_type); - } - - if (has_optional) print_string(PRINT_FP, NULL, "%s\t", _SL_); + } if (tb[TCA_IFE_METALST]) { struct rtattr *metalist[IFE_META_MAX + 1]; @@ -290,21 +286,17 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) } - if (tb[TCA_IFE_DMAC]) { - has_optional = 1; + if (tb[TCA_IFE_DMAC]) print_string(PRINT_ANY, "dst", "dst %s ", ll_addr_n2a(RTA_DATA(tb[TCA_IFE_DMAC]), RTA_PAYLOAD(tb[TCA_IFE_DMAC]), 0, b2, sizeof(b2))); - } - if (tb[TCA_IFE_SMAC]) { - has_optional = 1; + if (tb[TCA_IFE_SMAC]) print_string(PRINT_ANY, "src", "src %s ", ll_addr_n2a(RTA_DATA(tb[TCA_IFE_SMAC]), RTA_PAYLOAD(tb[TCA_IFE_SMAC]), 0, b2, sizeof(b2))); - } print_nl(); print_uint(PRINT_ANY, "index", "\t index %u", p->index);
On Mon, Feb 19, 2024 at 12:59 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Sun, 18 Feb 2024 22:44:13 +0300 > Maks Mishin <maks.mishinfz@gmail.com> wrote: > > > The variable `has_optional` do not used after set the value. > > Found by RASU JSC. > > > > Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> > > Yes, it is set after used. > The flag should be removed all together and fold the newline into > previous clause? > > And since the if clauses are now single statement, the code style > from checkpatch wants braces to be removed. > > > Something like this: > Yes, this looks better. Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal > diff --git a/tc/m_ife.c b/tc/m_ife.c > index 162607ce7415..fce5af784e39 100644 > --- a/tc/m_ife.c > +++ b/tc/m_ife.c > @@ -219,7 +219,6 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) > __u32 mmark = 0; > __u16 mtcindex = 0; > __u32 mprio = 0; > - int has_optional = 0; > SPRINT_BUF(b2); > > print_string(PRINT_ANY, "kind", "%s ", "ife"); > @@ -240,12 +239,9 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) > > if (tb[TCA_IFE_TYPE]) { > ife_type = rta_getattr_u16(tb[TCA_IFE_TYPE]); > - has_optional = 1; > print_0xhex(PRINT_ANY, "type", "type %#llX ", ife_type); > - } > - > - if (has_optional) > print_string(PRINT_FP, NULL, "%s\t", _SL_); > + } > > if (tb[TCA_IFE_METALST]) { > struct rtattr *metalist[IFE_META_MAX + 1]; > @@ -290,21 +286,17 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) > > } > > - if (tb[TCA_IFE_DMAC]) { > - has_optional = 1; > + if (tb[TCA_IFE_DMAC]) > print_string(PRINT_ANY, "dst", "dst %s ", > ll_addr_n2a(RTA_DATA(tb[TCA_IFE_DMAC]), > RTA_PAYLOAD(tb[TCA_IFE_DMAC]), 0, b2, > sizeof(b2))); > - } > > - if (tb[TCA_IFE_SMAC]) { > - has_optional = 1; > + if (tb[TCA_IFE_SMAC]) > print_string(PRINT_ANY, "src", "src %s ", > ll_addr_n2a(RTA_DATA(tb[TCA_IFE_SMAC]), > RTA_PAYLOAD(tb[TCA_IFE_SMAC]), 0, b2, > sizeof(b2))); > - } > > print_nl(); > print_uint(PRINT_ANY, "index", "\t index %u", p->index); >
diff --git a/tc/m_ife.c b/tc/m_ife.c index 162607ce..42621bec 100644 --- a/tc/m_ife.c +++ b/tc/m_ife.c @@ -291,7 +291,6 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) } if (tb[TCA_IFE_DMAC]) { - has_optional = 1; print_string(PRINT_ANY, "dst", "dst %s ", ll_addr_n2a(RTA_DATA(tb[TCA_IFE_DMAC]), RTA_PAYLOAD(tb[TCA_IFE_DMAC]), 0, b2, @@ -299,7 +298,6 @@ static int print_ife(struct action_util *au, FILE *f, struct rtattr *arg) } if (tb[TCA_IFE_SMAC]) { - has_optional = 1; print_string(PRINT_ANY, "src", "src %s ", ll_addr_n2a(RTA_DATA(tb[TCA_IFE_SMAC]), RTA_PAYLOAD(tb[TCA_IFE_SMAC]), 0, b2,
The variable `has_optional` do not used after set the value. Found by RASU JSC. Signed-off-by: Maks Mishin <maks.mishinFZ@gmail.com> --- tc/m_ife.c | 2 -- 1 file changed, 2 deletions(-)