linux-kernel - Re: [PATCH] removes unused variable from kernel/sysctl.h

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Apr 2009 10:59:56 +0530
From:	Sukanto Ghosh <sukanto.cse.iitb@...il.com>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
Cc:	trivial@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] removes unused variable from kernel/sysctl.h

Hi,

On Fri, Apr 17, 2009 at 12:36 AM, Jaswinder Singh Rajput
<jaswinder@...nel.org> wrote:
> Hello Sukanto,
>
> On Thu, 2009-04-16 at 23:22 +0530, Sukanto Ghosh wrote:
>> This patch removes the unused variable 'val' from the
>> __do_proc_dointvec() function
>> in kernel/sysctl.h. The integer has been declared and used as 'val =
>> -val' and there is
>> no reference to it anywhere.
>> Although I am this doesn't affects the kernel binary because gcc
>> removes it, still it
>> might be confusing for people reading the code.
>>
>
> This patch is already submitted by Tomohiro Kusumi on Thu Dec 27 2007:
>
> http://lkml.indiana.edu/hypermail/linux/kernel/0712.3/0557.html

I asked Tomohiro regarding this and got the following reply:

----
I think they just missed that patch since I found my name on
the following page regarding this patch.
http://lkml.org/lkml/2008/5/30/335

I think it's obvious that the val is unnecessary and someone
probably just forgot to remove it when they cleaned up that code.
-----

>
>>
>> Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@...il.com>
>>
>> ----
>>
>> --- linux-2.6.29.1/kernel/sysctl.c.orig 2009-04-16 19:57:21.000000000 +0530
>> +++ linux-2.6.29.1/kernel/sysctl.c      2009-04-16 19:58:26.000000000 +0530
>> @@ -2198,7 +2198,7 @@ static int __do_proc_dointvec(void *tbl_
>>                   void *data)
>
> This is a very old function and have almost no git history, its old name
> was do_proc_dointvec on 2005-04-16 15:20:36 when git tree was made and
> then only(almost) function name is changed.
>
>>  {
>>  #define TMPBUFLEN 21
>> -       int *i, vleft, first=1, neg, val;
>> +       int *i, vleft, first=1, neg;
>>         unsigned long lval;
>>         size_t left, len;
>>
>> @@ -2251,8 +2251,6 @@ static int __do_proc_dointvec(void *tbl_
>>                         len = p-buf;
>>                         if ((len < left) && *p && !isspace(*p))
>>                                 break;
>> -                       if (neg)
>> -                               val = -val;
>
> So you need to do further investigation. Is this a typo or some other
> mistake.

Considering that this function is currently working properly and the
variable 'val' is not used anywhere, it is obvious that removing the
variable will not affect the functionality of the function in any
manner.

It seems that either people haven't noticed this unused variable
before Tomohiro did, or they did not see merit in submitting the
trivial patch. But I believe such code cleanups are required.

If you (maintainers) feel that it is worth it, should I re-submit the patch ?


-- 
Regards,
Sukanto Ghosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/