[ovs-dev] [PATCH] dpif-netdev: Fix integer handling issue.
Ilya Maximets
i.maximets at ovn.org
Wed Mar 18 14:59:23 UTC 2020
On 3/18/20 12:31 AM, William Tu wrote:
> Coverity CID 279497 reports "Operands don't affect result".
> Because flow->ct_state is uint8_t and DP_NETDEV_CS_UNSUPPORTED_MASK
> is '0xffffff00'. So remove the statement.
>
> Cc: Usman Ansari <uansari at vmware.com>
> Signed-off-by: William Tu <u9012063 at gmail.com>
> ---
> lib/dpif-netdev.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a798db45d9cb..0e2678d002d5 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3224,10 +3224,6 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
> return EINVAL;
> }
>
> - if (flow->ct_state & DP_NETDEV_CS_UNSUPPORTED_MASK) {
> - return EINVAL;
> - }
> -
I'm not sure if we need to remove this. This code doesn't make any harm
and most likely compiled out. I agree that it doesn't change any logic
in this function, but in case someone will try to add new flags or change
the type of ct_state we will be safe and will reject all the unknown flags.
Without this code we'll have to catch this case somehow on code review and
re-introduce this check or implement missing functionality.
One more thing is that DP_NETDEV_CS_UNSUPPORTED_MASK definition becomes
unused and should be removed along with _SUPPORTED_MASK.
So, I'd rather not touch this and just mark this code as OK for coverity
scanner. But if you want to remove, please, clean up other parts and
add a build assert for the ct_state size and flags, so any disruptive change
will be caught by the developer of this change.
Best regards, Ilya Maximets.
More information about the dev
mailing list