Re: [PATCH] shared: linux: remove unused variables

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, Sep 23, 2024 at 10:37:43AM GMT, Sid Kumar wrote:
>
> On 9/2/24 11:29 PM, Ba Jing wrote:
> > These variables are never referenced in the code, just remove them.
>
> Hello,
>
> I think these variables are needed and they are referenced in main.c within
> the radix-tree directory. With these lines removed and running make within
> tools/testing/radix-tree I get the following errors:
>
> /usr/bin/ld: /home/ubuntu/linux-sidhakum-public/tools/testing/radix-tree/main.c:74:(.text+0x57a):
> undefined reference to `test_verbose'
> /usr/bin/ld: main.o: in function `check_copied_tags':
> /home/ubuntu/linux-sidhakum-public/tools/testing/radix-tree/main.c:135:(.text+0xc3f):
> undefined reference to `test_verbose'
>
>
> /home/ubuntu/linux-sidhakum-public/tools/testing/radix-tree/main.c:243:(.text+0x1d68):
> undefined reference to `preempt_count'
> /usr/bin/ld: /home/ubuntu/linux-sidhakum-public/tools/testing/radix-tree/main.c:243:(.text+0x1d8d):
> undefined reference to `preempt_count'
>
> adding back these variables allows make to run successfully again.
>
> Thanks,
>
> Sidhartha Kumar

Agreed, this patch is just incorrect, obviously the above that is required
to demonstrate that, but a quick grep also shows these variables in use.

It's important when performing a patch removing things like this to check
carefully that you are not mistaken, a quick grep is a basic step (but not
always sufficient as the C preprocessor might generate variable names
etc. so you should be _absolutely_ sure).

Also, generally speaking, there might be reasons for maintaing variables
even if they _are_ unused. We've had a number of people sending patches
trying to remove _actually_ unused variables which were incorrect.

So, I'd suggest in future if submitting a similar kind of patch, to send it
as an RFC, so you first get input on this.

Though again in this case, this is simply incorrect.
>
>
> >
> > Signed-off-by: Ba Jing <bajing@cmss.chinamobile.com>
> > ---
> >   tools/testing/shared/linux.c | 2 --
> >   1 file changed, 2 deletions(-)
> >
> > diff --git a/tools/testing/shared/linux.c b/tools/testing/shared/linux.c
> > index 17263696b5d8..eb1b1878cf64 100644
> > --- a/tools/testing/shared/linux.c
> > +++ b/tools/testing/shared/linux.c
> > @@ -13,8 +13,6 @@
> >   #include <urcu/uatomic.h>
> >   int nr_allocated;
> > -int preempt_count;
> > -int test_verbose;
> >   struct kmem_cache {
> >   	pthread_mutex_t lock;