| 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
| ||
|
Message-ID: <20240806082558.ytq673mhuji32koz@quack3>
Date: Tue, 6 Aug 2024 10:25:58 +0200
From: Jan Kara <jack@...e.cz>
To: Mirsad Todorovac <mtodorovac69@...il.com>
Cc: Jan Kara <jack@...e.cz>,
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 Mon 05-08-24 23:24:06, Mirsad Todorovac wrote:
> On 8/5/24 15:04, Jan Kara wrote:
> > On Fri 02-08-24 18:31:46, Mirsad Todorovac wrote:
> >> 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
> >>
> >> Hi, Jan,
> >>
> >> After a short break, I just tried a full build with this hack against the vanilla
> >> linux-next tree:
> >>
> >> #define RFALSE( cond, format, args... ) do { (void)(cond); } while( 0 )
> >>
> >> and it breaks at least here:
> >>
> >> In file included from fs/reiserfs/do_balan.c:15:
> >> fs/reiserfs/do_balan.c: In function ‘balance_leaf_when_delete_del’:
> >> fs/reiserfs/do_balan.c:86:28: error: ‘ih’ undeclared (first use in this function)
> >> 86 | RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
> >> | ^~
> >> fs/reiserfs/reiserfs.h:919:54: note: in definition of macro ‘RFALSE’
> >> 919 | #define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> >> | ^~~~
> >> ./include/linux/byteorder/generic.h:91:21: note: in expansion of macro ‘__le16_to_cpu’
> >> 91 | #define le16_to_cpu __le16_to_cpu
> >> | ^~~~~~~~~~~~~
> >> fs/reiserfs/do_balan.c:86:16: note: in expansion of macro ‘ih_item_len’
> >> 86 | RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
> >> | ^~~~~~~~~~~
> >> fs/reiserfs/do_balan.c:86:28: note: each undeclared identifier is reported only once for each function it appears in
> >> 86 | RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
> >> | ^~
> >> fs/reiserfs/reiserfs.h:919:54: note: in definition of macro ‘RFALSE’
> >> 919 | #define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> >> | ^~~~
> >> ./include/linux/byteorder/generic.h:91:21: note: in expansion of macro ‘__le16_to_cpu’
> >> 91 | #define le16_to_cpu __le16_to_cpu
> >> | ^~~~~~~~~~~~~
> >> fs/reiserfs/do_balan.c:86:16: note: in expansion of macro ‘ih_item_len’
> >> 86 | RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
> >> | ^~~~~~~~~~~
> >> fs/reiserfs/do_balan.c: In function ‘do_balance_starts’:
> >> fs/reiserfs/do_balan.c:1800:16: error: implicit declaration of function ‘check_before_balancing’ [-Werror=implicit-function-declaration]
> >> 1800 | RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
> >> | ^~~~~~~~~~~~~~~~~~~~~~
> >> fs/reiserfs/reiserfs.h:919:54: note: in definition of macro ‘RFALSE’
> >> 919 | #define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> >> | ^~~~
> >> cc1: some warnings being treated as errors
> >> make[7]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
> >> CC [M] fs/reiserfs/stree.o
> >> In file included from fs/reiserfs/stree.c:15:
> >> fs/reiserfs/stree.c: In function ‘reiserfs_delete_item’:
> >> fs/reiserfs/stree.c:1283:24: error: ‘mode’ undeclared (first use in this function)
> >> 1283 | RFALSE(mode != M_DELETE, "PAP-5320: mode must be M_DELETE");
> >> | ^~~~
> >> fs/reiserfs/reiserfs.h:919:54: note: in definition of macro ‘RFALSE’
> >> 919 | #define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> >> | ^~~~
> >> fs/reiserfs/stree.c:1283:24: note: each undeclared identifier is reported only once for each function it appears in
> >> 1283 | RFALSE(mode != M_DELETE, "PAP-5320: mode must be M_DELETE");
> >> | ^~~~
> >> fs/reiserfs/reiserfs.h:919:54: note: in definition of macro ‘RFALSE’
> >> 919 | #define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> >> | ^~~~
> >>
> >> Last time it compiled, but now it expects variables in (void)(cond) expressions to be defined.
> >>
> >> I have try to fix those warnings, submitting the patch for review:
> >
> > Yeah, this looks ok to me.
> >
> > Honza
>
> Hi, Jan,
>
> As I understood from the previous experiences, and the Code of Conduct,
> and explicit Reviwed-by: or Acked-by: is required ...
>
> Or otherwise, the Suggested-by: is used?
So I was waiting for you to submit official patch with proper changelog and
your Signed-off-by. Then I can pick up the patch into my tree and merge it.
Honza
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists