Re: [PATCH rdma-next] RDMA/mlx5: Init dest_type when create flow

[Jason Gunthorpe <jgg@nvidia.com>]

On Sun, Jul 12, 2020 at 08:40:16PM +0300, Leon Romanovsky wrote:
> On Fri, Jul 10, 2020 at 04:46:44PM -0300, Jason Gunthorpe wrote:
> > On Tue, Jul 07, 2020 at 02:02:59PM +0300, Leon Romanovsky wrote:
> > > From: Daria Velikovsky <daria@mellanox.com>
> > >
> > > When using action drop dest_type was never assigned to any value.
> > > Add initialization of dest_type to -1 since 0 is valid.
> > >
> > > Fixes: f29de9eee782 ("RDMA/mlx5: Add support for drop action in DV steering")
> > > Signed-off-by: Daria Velikovsky <daria@mellanox.com>
> > > Reviewed-by: Maor Gottlieb <maorg@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > >  Based on
> > > https://lore.kernel.org/lkml/20200702081809.423482-1-leon@kernel.org
> > >  drivers/infiniband/hw/mlx5/fs.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c
> > > index 0d8abb7c3cdf..1a7e6226f11a 100644
> > > +++ b/drivers/infiniband/hw/mlx5/fs.c
> > > @@ -1903,7 +1903,7 @@ static int UVERBS_HANDLER(MLX5_IB_METHOD_CREATE_FLOW)(
> > >  	struct mlx5_flow_context flow_context = {.flow_tag =
> > >  		MLX5_FS_DEFAULT_FLOW_TAG};
> > >  	u32 *offset_attr, offset = 0, counter_id = 0;
> > > -	int dest_id, dest_type, inlen, len, ret, i;
> > > +	int dest_id, dest_type = -1, inlen, len, ret, i;
> >
> > I think this should be done inside get_dests() - it is pretty ugly to
> > have an function with an output pointer that is only filled sometimes
> > on success.
> 
> This was original patch which I rewrote because don't like the approach
> when function changes fields when it doesn't need to change. I prefer
> the current approach where caller has explicitly decided which default
> value he wants.

How is it a "default value" ? The function's job is to fill dest_type,
it should fill it correctly or fail

Jason