On Wed, Mar 18, 2020 at 8:36 AM Ben Pfaff <[email protected]> wrote: > > On Wed, Mar 18, 2020 at 08:34:16AM -0700, William Tu wrote: > > On Wed, Mar 18, 2020 at 7:59 AM Ilya Maximets <[email protected]> wrote: > > > > > > 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 <[email protected]> > > > > Signed-off-by: William Tu <[email protected]> > > > > --- > > > > 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. > > > > Good point. > > > > > > > > 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. > > > > > OK thanks! > > Let's keep this code block as it is now. > > I was surprised to hear that it doesn't have any effect. Adding a > comment might be helpful.
OK, then let me just add a comment. In case in the future people run Coverity and see this again. William _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
