linux-kernel - Re: linux-next: build failure after merge of the xarray tree

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20190311141235.GH22862@mellanox.com>
Date:   Mon, 11 Mar 2019 14:12:39 +0000
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     Leon Romanovsky <leonro@...lanox.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Doug Ledford <dledford@...hat.com>,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: linux-next: build failure after merge of the xarray tree

On Mon, Mar 11, 2019 at 05:31:05AM -0700, Matthew Wilcox wrote:
> On Mon, Mar 11, 2019 at 12:13:54PM +0000, Jason Gunthorpe wrote:
> > > @@ -1059,14 +1050,14 @@ static int assign_client_id(struct ib_client *client)
> > >  	 * to get the LIFO order. The extra linked list can go away if xarray
> > >  	 * learns to reverse iterate.
> > >  	 */
> > > -	if (list_empty(&client_list))
> > > +	if (list_empty(&client_list)) {
> > >  		client->client_id = 0;
> > > -	else
> > > -		client->client_id =
> > > -			list_last_entry(&client_list, struct ib_client, list)
> > > -				->client_id;
> > > -	ret = xa_alloc(&clients, &client->client_id, INT_MAX, client,
> > > -		       GFP_KERNEL);
> > > +	} else {
> > > +		struct ib_client *last = list_last_entry(&client_list,
> > > +				struct ib_client, list);
> > > +		client->client_id = last->client_id + 1;
> > 
> > blank line after locals, but other wise these all looks fine.. 
> 
> Would you rather see this rendered as:
> 
>         if (list_empty(&client_list)) {
>                 client->client_id = 0;
>         } else {
>                 struct ib_client *last; 
> 
>                 last = list_last_entry(&client_list, struct ib_client, list);
>                 client->client_id = last->client_id + 1;
>         }

Don't care much either way. Only that the Linux style guide is to
always have the blank line after variable declarations in any block

> or move the declaration of 'last' up to the top of the function?

This one I dislike, variables should be in their narrowest scope for
clarity

> > Should have started out with the xa_insert version above..
> 
> I didn't spot it until last night either ...

It is a leftover weird thinking logic from an earlier attempt I had
that was trying to get the last ID out of the xarray without the
linked list..

Jason