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

[Mirsad Todorovac <mtodorovac69@gmail.com>]

On 7/18/24 11:39, Jan Kara wrote:
> On Thu 18-07-24 00:14:24, Mirsad Todorovac wrote:
>>
>>
>> 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.
> 
> Well, there was also ext3 at that time already :-) That's where I became
> familiar with the idea of journalling. Reiserfs was interesting to me
> because of completely different approach to on-disk format (b-tree with
> formatted items), packing of small files / file tails (interesting in 2000,
> not so much 20 years later) and reasonable scalability for large
> directories.
> 
>> I think your idea is great and if you wish a patch could be submitted
>> after the merge window.
> 
> I'll leave it up to you. If the warnings annoy you, send the patch along
> the lines I've proposed (BTW (void)cond should better be (void)(cond) for
> macro safety) and I'll push it to Linus.
> 
> 								Honza

Sure thing. Yes, (ovid)(cond) makes much more sense against i.e.
expanding RFALSE(a + b, ...).

Best regards,
Mirsad Todorovac