On Thu, Sep 6, 2018 at 7:09 AM, Leon Romanovsky <leonro@xxxxxxxxxxxx> wrote: > 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. slow down, I was just supporting Jason's suggestion and said there's no reason not to follow it. If you don't agree with Jason, argue with him. > kernel git:(rdma-next) git grep "\!\!" |wc -l > 8125
