netdev - Re: [PATCH mlx5-next v1 05/15] net/mlx5: Break encap/decap into two separated flow table creation flags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Sep 2018 07:09:50 +0300
From:   Leon Romanovsky <leonro@...lanox.com>
To:     Or Gerlitz <gerlitz.or@...il.com>
Cc:     Jason Gunthorpe <jgg@...pe.ca>, Doug Ledford <dledford@...hat.com>,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Ariel Levkovich <lariel@...lanox.com>,
        Mark Bloch <markb@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH mlx5-next v1 05/15] net/mlx5: Break encap/decap into two
 separated flow table creation flags

On Thu, Sep 06, 2018 at 12:37:17AM +0300, Or Gerlitz wrote:
> On Wed, Sep 5, 2018 at 9:11 PM, Leon Romanovsky  wrote:
> > On Wed, Sep 05, 2018 at 10:38:00AM -0600, Jason Gunthorpe wrote:
> >> On Wed, Sep 05, 2018 at 08:10:25AM +0300, Leon Romanovsky wrote:
> >> > > > -       int en_encap_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN);
> >> > > > +       int en_encap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_ENCAP);
> >> > > > +       int en_decap = !!(flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP);
> >> > >
> >> > > Yuk, please don't use !!.
> >> > >
> >> > >   bool en_decap = flags & MLX5_FLOW_TABLE_TUNNEL_EN_DECAP;
> >> >
> >> > We need to provide en_encap and en_decap as an input to MLX5_SET(...)
> >> > which is passed to FW as 0 or 1.
> >> >
> >> > Boolean type is declared in C as int and treated as zero for false
> >> > and any other value for true,
> >>
> >> No, that isn't right, the kernel uses C99's _Bool intrinsic type, which
> >> is guaranteed to only hold 0 or 1 by the compiler.
> >>
> >> See types.h:
> >>
> >> typedef _Bool                   bool;
> >
> > Exciting, it took me a while to find C99 standard and relevant 6.3.1.2.
> > Anyway, this patch didn't change previous functionality, which used "!!"
> > convention.
>
> so? if we didn't do things properly prior to the patch, why not fixing it along
> with the patch? lets fix

Or,

What exactly "to fix"? Both code lines:
1. Have correct syntax
2. Implement proper C99
3. Give same compiler code
4. Have same readability

There is nothing to fix.

And this patch is already merged, so if you truly care about this,
please go ahead and prepare patch for whole driver, or better for
whole kernel.

 kernel git:(rdma-next) git grep "\!\!" |wc -l
8125

Thanks

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)