linux-kernel - Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]

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]

Message-ID: <83c22d71-8706-4779-8d20-6b18a75c95a5@gmail.com>
Date: Thu, 18 Jul 2024 00:14:24 +0200
From: Mirsad Todorovac <mtodorovac69@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
 "Gustavo A. R. Silva" <gustavoars@...nel.org>, Kees Cook <kees@...nel.org>,
 Christian Brauner <brauner@...nel.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Al Viro <viro@...iv.linux.org.uk>, Jeff Layton <jlayton@...nel.org>,
 reiserfs-devel@...r.kernel.org
Subject: Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]



On 7/17/24 17:44, Jan Kara wrote:
> On Tue 16-07-24 19:17:05, Mirsad Todorovac wrote:
>> On 7/15/24 19:28, Jan Kara wrote:
>>> Hello Mirsad!
>>>
>>> On Wed 10-07-24 20:09:27, Mirsad Todorovac wrote:
>>>> On the linux-next vanilla next-20240709 tree, I have attempted the seed KCONFIG_SEED=0xEE7AB52F
>>>> which was known from before to trigger various errors in compile and build process.
>>>>
>>>> Though this might seem as contributing to channel noise, Linux refuses to build this config,
>>>> treating warnings as errors, using this build line:
>>>>
>>>> $ time nice make W=1 -k -j 36 |& tee ../err-next-20230709-01a.log; date
>>>>
>>>> As I know that the Chief Penguin doesn't like warnings, but I am also aware that there are plenty
>>>> left, there seems to be more tedious work ahead to make the compilers happy.
>>>>
>>>> The compiler output is:
>>>>
>>>> ---------------------------------------------------------------------------------------------------------
>>>> fs/reiserfs/do_balan.c: In function ‘balance_leaf_new_nodes_paste_whole’:
>>>> fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
>>>>  1147 |         int leaf_mi;
>>>>       |             ^~~~~~~
>>>
>>> Frankly, I wouldn't bother with reiserfs. The warning is there for ages,
>>> the code is going to get removed in two releases, so I guess we can live
>>> with these warnings for a few more months...
>>
>> In essence I agree with you, but for sentimental reasons I would like to
>> keep it because it is my first journaling Linux system on Knoppix 🙂
> 
> As much as I understand your sentiment (I have a bit of history with that
> fs as well) the maintenance cost isn't really worth it and most fs folks
> will celebrate when it's removed. We have already announced the removal
> year and half ago and I'm fully for executing that plan at the end of this
> year.
> 
>> Patch is also simple and a no-brainer, as proposed by Mr. Cook:
>>
>> -------------------------------><------------------------------------------
>> diff --git a/fs/reiserfs/do_balan.c b/fs/reiserfs/do_balan.c
>> index 5129efc6f2e6..fbe73f267853 100644
>> --- a/fs/reiserfs/do_balan.c
>> +++ b/fs/reiserfs/do_balan.c
>> @@ -1144,7 +1144,9 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
>>  {
>>  	struct buffer_head *tbS0 = PATH_PLAST_BUFFER(tb->tb_path);
>>  	int n = B_NR_ITEMS(tbS0);
>> +#ifdef CONFIG_REISERFS_CHECK
>>  	int leaf_mi;
>> +#endif
> 
> Well, I would not like this even for actively maintained code ;) If you
> want to silence these warnings in this dead code, then I could live with
> something like:
> 
> #if defined( CONFIG_REISERFS_CHECK )
> #define RFALSE(cond, format, args...) __RASSERT(!(cond), ....)
> #else
> - #define RFALSE( cond, format, args... ) do {;} while( 0 )
> + #define RFALSE( cond, format, args... ) do { (void)cond; } while( 0 )
> #endif

Yes, one line change is much smarter than 107 line patch of mine :-)

Verified, and this line solved all the warnings:

  CC      fs/reiserfs/bitmap.o
  CC      fs/reiserfs/do_balan.o
  CC      fs/reiserfs/namei.o
  CC      fs/reiserfs/inode.o
  CC      fs/reiserfs/file.o
  CC      fs/reiserfs/dir.o
  CC      fs/reiserfs/fix_node.o
  CC      fs/reiserfs/super.o
  CC      fs/reiserfs/prints.o
  CC      fs/reiserfs/objectid.o
  CC      fs/reiserfs/lbalance.o
  CC      fs/reiserfs/ibalance.o
  CC      fs/reiserfs/stree.o
  CC      fs/reiserfs/hashes.o
  CC      fs/reiserfs/tail_conversion.o
  CC      fs/reiserfs/journal.o
  CC      fs/reiserfs/resize.o
  CC      fs/reiserfs/item_ops.o
  CC      fs/reiserfs/ioctl.o
  CC      fs/reiserfs/xattr.o
  CC      fs/reiserfs/lock.o
  CC      fs/reiserfs/procfs.o
  AR      fs/reiserfs/built-in.a

Just FWIW, back then in year 2000/2001 a journaling file system on my Knoppix box was a quantum
leap - it would simply replay the journal if there was a power loss before shutdown. No several
minutes of fsck.

I think your idea is great and if you wish a patch could be submitted after the merge window.

Best regards,
Mirsad

> 
> 								Honza