Re: [PATCH v5 7/7] net/eth: Add an assert() and invert if() statement to simplify code

[Stefano Garzarella]

On Wed, Mar 10, 2021 at 05:01:35PM +0100, Philippe Mathieu-Daudé wrote:
> To simplify the function body, invert the if() statement, returning
> earlier.
> Since we already checked there is enough data in the iovec buffer,
> simply add an assert() call to consume the bytes_read variable.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> net/eth.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/net/eth.c b/net/eth.c
> index b150d73c13a..c0a5ca12be2 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -416,16 +416,14 @@ _eth_get_rss_ex_dst_addr(const struct iovec *pkt, int 
> pkt_frags,
>     bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset,
>                             &rt_hdr, sizeof(rt_hdr));
>     assert(bytes_read == sizeof(rt_hdr));
> -
> -    if ((rt_hdr.rtype == 2) && (rt_hdr.segleft == 1)) {
> -        bytes_read = iov_to_buf(pkt, pkt_frags,
> -                                ext_hdr_offset + sizeof(*ext_hdr),
> -                                dst_addr, sizeof(*dst_addr));
> -
> -        return bytes_read == sizeof(*dst_addr);
> +    if ((rt_hdr.rtype != 2) || (rt_hdr.segleft != 1)) {
> +        return false;
>     }
> +    bytes_read = iov_to_buf(pkt, pkt_frags, ext_hdr_offset + sizeof(rt_hdr),

If we add the offset fix in patch 5 this patch is fine, otherwise we 
should mention the fix here in the commit message (and queue this for 
stable?).

With that fixed:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano

> +                            dst_addr, sizeof(*dst_addr));
> +    assert(bytes_read == sizeof(*dst_addr));
>
> -    return false;
> +    return true;
> }
>
> static bool
> --
> 2.26.2