linux-kernel - Re: [PATCH] gnss: Remove usage of the deprecated ida_simple

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: <YrF0ozJgjCch7cC6@hovoldconsulting.com>
Date:   Tue, 21 Jun 2022 09:34:59 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     Bo Liu <liubo03@...pur.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] gnss: Remove usage of the deprecated ida_simple_xxx API

On Thu, Jun 16, 2022 at 09:18:37PM +0200, Christophe JAILLET wrote:
> Le 16/06/2022 à 06:47, Bo Liu a écrit :
> > Use ida_alloc_xxx()/ida_free() instead of
> > ida_simple_get()/ida_simple_remove().
> > The latter is deprecated and more verbose.
> > 
> > Signed-off-by: Bo Liu <liubo03@...pur.com>

> even if a more detailed description about the reason of the added -1 
> would be welcomed:
> 
> Reviewed-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>

I agree, that should have been mentioned. And thanks for reviewing.

> > ---
> >   drivers/gnss/core.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gnss/core.c b/drivers/gnss/core.c
> > index e6f94501cb28..1e82b7967570 100644
> > --- a/drivers/gnss/core.c
> > +++ b/drivers/gnss/core.c
> > @@ -217,7 +217,7 @@ static void gnss_device_release(struct device *dev)
> >   
> >   	kfree(gdev->write_buf);
> >   	kfifo_free(&gdev->read_fifo);
> > -	ida_simple_remove(&gnss_minors, gdev->id);
> > +	ida_free(&gnss_minors, gdev->id);
> >   	kfree(gdev);
> >   }
> >   
> > @@ -232,7 +232,7 @@ struct gnss_device *gnss_allocate_device(struct device *parent)
> >   	if (!gdev)
> >   		return NULL;
> >   
> > -	id = ida_simple_get(&gnss_minors, 0, GNSS_MINORS, GFP_KERNEL);
> > +	id = ida_alloc_max(&gnss_minors, GNSS_MINORS - 1, GFP_KERNEL);

I'm also not sure about the claim that the new interface is less
verbose.

The above line is exactly as long as the old one, but somewhat harder to
read due to the inclusive upper bound and the various allocator flavors
(ε, max, min, range).

> >   	if (id < 0) {
> >   		kfree(gdev);
> >   		return NULL;

Now applied with an amended commit message:

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/gnss.git/commit/?h=gnss-next&id=c0c725d7350ec8b8453257676a440bb4b2df2422

Johan