* [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
@ 2024-07-10 18:09 Mirsad Todorovac
2024-07-10 18:17 ` Kees Cook
2024-07-15 17:28 ` Jan Kara
0 siblings, 2 replies; 16+ messages in thread
From: Mirsad Todorovac @ 2024-07-10 18:09 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Gustavo A. R. Silva, Kees Cook, Christian Brauner, Jan Kara,
Matthew Wilcox (Oracle), Al Viro, Jeff Layton, reiserfs-devel
Dear all,
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;
| ^~~~~~~
CC fs/reiserfs/lbalance.o
fs/reiserfs/fix_node.c: In function ‘dc_check_balance_leaf’:
fs/reiserfs/fix_node.c:1938:13: error: variable ‘maxsize’ set but not used [-Werror=unused-but-set-variable]
1938 | int maxsize, ret;
| ^~~~~~~
fs/reiserfs/fix_node.c:1935:13: error: variable ‘levbytes’ set but not used [-Werror=unused-but-set-variable]
1935 | int levbytes;
| ^~~~~~~~
fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
221 | p += vscnprintf(p, end - p, fmt1, args);
| ^
fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
260 | p += vscnprintf(p, end - p, fmt1, args);
| ^
make[4]: Target 'arch/x86/kernel/' not remade because of errors.
make[3]: *** [scripts/Makefile.build:485: arch/x86/kernel] Error 2
make[3]: Target 'arch/x86/' not remade because of errors.
make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
fs/reiserfs/lbalance.c: In function ‘leaf_copy_items’:
fs/reiserfs/lbalance.c:524:29: error: variable ‘dest’ set but not used [-Werror=unused-but-set-variable]
524 | struct buffer_head *dest;
| ^~~~
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/prints.o] Error 1
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/fix_node.o] Error 1
---------------------------------------------------------------------------------------------------------
In fs/reiserfs/fix_node.c:1938:13, fs/reiserfs/fix_node.c:1935:13, and fs/reiserfs/lbalance.c:524:29,
the problem seem to lie within the construct RFALSE(), like
521 static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
522 int last_first, int cpy_num, int cpy_bytes)
523 {
524 struct buffer_head *dest;
525 int pos, i, src_nr_item, bytes;
526
527 dest = dest_bi->bi_bh;
528 RFALSE(!dest || !src, "vs-10210: !dest || !src");
529 RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
530 "vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
531 RFALSE(B_NR_ITEMS(src) < cpy_num,
532 "vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
533 cpy_num);
534 RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
Hope this helps.
Best regards,
Mirsad Todorovac
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-10 18:09 [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
@ 2024-07-10 18:17 ` Kees Cook
2024-07-10 19:14 ` Mirsad Todorovac
2024-07-15 17:28 ` Jan Kara
1 sibling, 1 reply; 16+ messages in thread
From: Kees Cook @ 2024-07-10 18:17 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Christian Brauner,
Jan Kara, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
On Wed, Jul 10, 2024 at 08:09:27PM +0200, Mirsad Todorovac wrote:
> Dear all,
>
> 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;
> | ^~~~~~~
> CC fs/reiserfs/lbalance.o
> fs/reiserfs/fix_node.c: In function ‘dc_check_balance_leaf’:
> fs/reiserfs/fix_node.c:1938:13: error: variable ‘maxsize’ set but not used [-Werror=unused-but-set-variable]
> 1938 | int maxsize, ret;
> | ^~~~~~~
> fs/reiserfs/fix_node.c:1935:13: error: variable ‘levbytes’ set but not used [-Werror=unused-but-set-variable]
> 1935 | int levbytes;
> | ^~~~~~~~
> fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
> fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> 221 | p += vscnprintf(p, end - p, fmt1, args);
> | ^
> fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> 260 | p += vscnprintf(p, end - p, fmt1, args);
> | ^
> make[4]: Target 'arch/x86/kernel/' not remade because of errors.
> make[3]: *** [scripts/Makefile.build:485: arch/x86/kernel] Error 2
> make[3]: Target 'arch/x86/' not remade because of errors.
> make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
> fs/reiserfs/lbalance.c: In function ‘leaf_copy_items’:
> fs/reiserfs/lbalance.c:524:29: error: variable ‘dest’ set but not used [-Werror=unused-but-set-variable]
> 524 | struct buffer_head *dest;
> | ^~~~
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/prints.o] Error 1
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/fix_node.o] Error 1
> ---------------------------------------------------------------------------------------------------------
>
> In fs/reiserfs/fix_node.c:1938:13, fs/reiserfs/fix_node.c:1935:13, and fs/reiserfs/lbalance.c:524:29,
> the problem seem to lie within the construct RFALSE(), like
>
> 521 static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
> 522 int last_first, int cpy_num, int cpy_bytes)
> 523 {
> 524 struct buffer_head *dest;
> 525 int pos, i, src_nr_item, bytes;
> 526
> 527 dest = dest_bi->bi_bh;
> 528 RFALSE(!dest || !src, "vs-10210: !dest || !src");
> 529 RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
> 530 "vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
> 531 RFALSE(B_NR_ITEMS(src) < cpy_num,
> 532 "vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
> 533 cpy_num);
> 534 RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
Can you prepare a patch to solve these? It should be possible to just
wrap the offending variables as done for RFALSE itself:
#ifdef CONFIG_REISERFS_CHECK
struct buffer_head *dest;
#endif
etc...
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-10 18:17 ` Kees Cook
@ 2024-07-10 19:14 ` Mirsad Todorovac
0 siblings, 0 replies; 16+ messages in thread
From: Mirsad Todorovac @ 2024-07-10 19:14 UTC (permalink / raw)
To: Kees Cook
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Christian Brauner,
Jan Kara, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
On 7/10/24 20:17, Kees Cook wrote:
> On Wed, Jul 10, 2024 at 08:09:27PM +0200, Mirsad Todorovac wrote:
>> Dear all,
>>
>> 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;
>> | ^~~~~~~
>> CC fs/reiserfs/lbalance.o
>> fs/reiserfs/fix_node.c: In function ‘dc_check_balance_leaf’:
>> fs/reiserfs/fix_node.c:1938:13: error: variable ‘maxsize’ set but not used [-Werror=unused-but-set-variable]
>> 1938 | int maxsize, ret;
>> | ^~~~~~~
>> fs/reiserfs/fix_node.c:1935:13: error: variable ‘levbytes’ set but not used [-Werror=unused-but-set-variable]
>> 1935 | int levbytes;
>> | ^~~~~~~~
>> fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
>> fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
>> 221 | p += vscnprintf(p, end - p, fmt1, args);
>> | ^
>> fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
>> 260 | p += vscnprintf(p, end - p, fmt1, args);
>> | ^
>> make[4]: Target 'arch/x86/kernel/' not remade because of errors.
>> make[3]: *** [scripts/Makefile.build:485: arch/x86/kernel] Error 2
>> make[3]: Target 'arch/x86/' not remade because of errors.
>> make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
>> fs/reiserfs/lbalance.c: In function ‘leaf_copy_items’:
>> fs/reiserfs/lbalance.c:524:29: error: variable ‘dest’ set but not used [-Werror=unused-but-set-variable]
>> 524 | struct buffer_head *dest;
>> | ^~~~
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/prints.o] Error 1
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/fix_node.o] Error 1
>> ---------------------------------------------------------------------------------------------------------
>>
>> In fs/reiserfs/fix_node.c:1938:13, fs/reiserfs/fix_node.c:1935:13, and fs/reiserfs/lbalance.c:524:29,
>> the problem seem to lie within the construct RFALSE(), like
>>
>> 521 static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
>> 522 int last_first, int cpy_num, int cpy_bytes)
>> 523 {
>> 524 struct buffer_head *dest;
>> 525 int pos, i, src_nr_item, bytes;
>> 526
>> 527 dest = dest_bi->bi_bh;
>> 528 RFALSE(!dest || !src, "vs-10210: !dest || !src");
>> 529 RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
>> 530 "vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
>> 531 RFALSE(B_NR_ITEMS(src) < cpy_num,
>> 532 "vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
>> 533 cpy_num);
>> 534 RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
>
> Can you prepare a patch to solve these? It should be possible to just
> wrap the offending variables as done for RFALSE itself:
>
> #ifdef CONFIG_REISERFS_CHECK
> struct buffer_head *dest;
> #endif
>
> etc...
Hi, Mr. Kees,
Well, i sort fo did it but I am not happy with it (it is not elegant like the original code):
-----><------------------
$ git diff fs/reiserfs
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
struct item_head *pasted;
struct buffer_info bi;
@@ -1157,7 +1159,6 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
reiserfs_panic(tb->tb_sb,
"PAP-12235",
"pos_in_item must be equal to ih_item_len");
-#endif
leaf_mi = leaf_move_items(LEAF_FROM_S_TO_SNEW, tb, tb->snum[i],
tb->sbytes[i], tb->S_new[i]);
@@ -1165,6 +1166,7 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
RFALSE(leaf_mi,
"PAP-12240: unexpected value returned by leaf_move_items (%d)",
leaf_mi);
+#endif
/* paste into item */
buffer_info_init_bh(tb, &bi, tb->S_new[i]);
diff --git a/fs/reiserfs/fix_node.c b/fs/reiserfs/fix_node.c
index 6c13a8d9a73c..0ad41751eca5 100644
--- a/fs/reiserfs/fix_node.c
+++ b/fs/reiserfs/fix_node.c
@@ -1926,6 +1926,7 @@ static int dc_check_balance_leaf(struct tree_balance *tb, int h)
{
struct virtual_node *vn = tb->tb_vn;
+#ifdef CONFIG_REISERFS_CHECK
/*
* Number of bytes that must be deleted from
* (value is negative if bytes are deleted) buffer which
@@ -1935,21 +1936,32 @@ static int dc_check_balance_leaf(struct tree_balance *tb, int h)
int levbytes;
/* the maximal item size */
- int maxsize, ret;
+ int maxsize;
+#endif
/*
* S0 is the node whose balance is currently being checked,
* and F0 is its father.
*/
- struct buffer_head *S0, *F0;
+
+#ifdef CONFIG_REISERFS_CHECK
+ struct buffer_head *S0;
+#endif
+ struct buffer_head *F0;
+
int lfree, rfree /* free space in L and R */ ;
+ int ret;
+#ifdef CONFIG_REISERFS_CHECK
S0 = PATH_H_PBUFFER(tb->tb_path, 0);
+#endif
F0 = PATH_H_PPARENT(tb->tb_path, 0);
+#ifdef CONFIG_REISERFS_CHECK
levbytes = tb->insert_size[h];
maxsize = MAX_CHILD_SIZE(S0); /* maximal possible size of an item */
+#endif
if (!F0) { /* S[0] is the root now. */
diff --git a/fs/reiserfs/lbalance.c b/fs/reiserfs/lbalance.c
index 7f868569d4d0..aa8d897368da 100644
--- a/fs/reiserfs/lbalance.c
+++ b/fs/reiserfs/lbalance.c
@@ -521,10 +521,14 @@ static void leaf_item_bottle(struct buffer_info *dest_bi,
static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
int last_first, int cpy_num, int cpy_bytes)
{
+#ifdef CONFIG_REISERFS_CHECK
struct buffer_head *dest;
+#endif
int pos, i, src_nr_item, bytes;
+#ifdef CONFIG_REISERFS_CHECK
dest = dest_bi->bi_bh;
+#endif
RFALSE(!dest || !src, "vs-10210: !dest || !src");
RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
"vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
--
P.S.
I could not find a mitigation for these:
fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
221 | p += vscnprintf(p, end - p, fmt1, args);
| ^
fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
260 | p += vscnprintf(p, end - p, fmt1, args);
| ^
221 p += vscnprintf(p, end - p, fmt1, args);
Hope this helps.
Best regards,
Mirsad Todorovac
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-10 18:09 [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
2024-07-10 18:17 ` Kees Cook
@ 2024-07-15 17:28 ` Jan Kara
2024-07-16 17:17 ` Mirsad Todorovac
1 sibling, 1 reply; 16+ messages in thread
From: Jan Kara @ 2024-07-15 17:28 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Jan Kara, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
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...
Honza
> CC fs/reiserfs/lbalance.o
> fs/reiserfs/fix_node.c: In function ‘dc_check_balance_leaf’:
> fs/reiserfs/fix_node.c:1938:13: error: variable ‘maxsize’ set but not used [-Werror=unused-but-set-variable]
> 1938 | int maxsize, ret;
> | ^~~~~~~
> fs/reiserfs/fix_node.c:1935:13: error: variable ‘levbytes’ set but not used [-Werror=unused-but-set-variable]
> 1935 | int levbytes;
> | ^~~~~~~~
> fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
> fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> 221 | p += vscnprintf(p, end - p, fmt1, args);
> | ^
> fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> 260 | p += vscnprintf(p, end - p, fmt1, args);
> | ^
> make[4]: Target 'arch/x86/kernel/' not remade because of errors.
> make[3]: *** [scripts/Makefile.build:485: arch/x86/kernel] Error 2
> make[3]: Target 'arch/x86/' not remade because of errors.
> make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
> fs/reiserfs/lbalance.c: In function ‘leaf_copy_items’:
> fs/reiserfs/lbalance.c:524:29: error: variable ‘dest’ set but not used [-Werror=unused-but-set-variable]
> 524 | struct buffer_head *dest;
> | ^~~~
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/prints.o] Error 1
> cc1: all warnings being treated as errors
> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/fix_node.o] Error 1
> ---------------------------------------------------------------------------------------------------------
>
> In fs/reiserfs/fix_node.c:1938:13, fs/reiserfs/fix_node.c:1935:13, and fs/reiserfs/lbalance.c:524:29,
> the problem seem to lie within the construct RFALSE(), like
>
> 521 static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
> 522 int last_first, int cpy_num, int cpy_bytes)
> 523 {
> 524 struct buffer_head *dest;
> 525 int pos, i, src_nr_item, bytes;
> 526
> 527 dest = dest_bi->bi_bh;
> 528 RFALSE(!dest || !src, "vs-10210: !dest || !src");
> 529 RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
> 530 "vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
> 531 RFALSE(B_NR_ITEMS(src) < cpy_num,
> 532 "vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
> 533 cpy_num);
> 534 RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
>
> Hope this helps.
>
> Best regards,
> Mirsad Todorovac
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-15 17:28 ` Jan Kara
@ 2024-07-16 17:17 ` Mirsad Todorovac
2024-07-17 15:44 ` Jan Kara
0 siblings, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-07-16 17:17 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
Hi Jan!
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 🙂
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
struct item_head *pasted;
struct buffer_info bi;
@@ -1157,7 +1159,6 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
reiserfs_panic(tb->tb_sb,
"PAP-12235",
"pos_in_item must be equal to ih_item_len");
-#endif
leaf_mi = leaf_move_items(LEAF_FROM_S_TO_SNEW, tb, tb->snum[i],
tb->sbytes[i], tb->S_new[i]);
@@ -1165,6 +1166,7 @@ static void balance_leaf_new_nodes_paste_whole(struct tree_balance *tb,
RFALSE(leaf_mi,
"PAP-12240: unexpected value returned by leaf_move_items (%d)",
leaf_mi);
+#endif
/* paste into item */
buffer_info_init_bh(tb, &bi, tb->S_new[i]);
diff --git a/fs/reiserfs/fix_node.c b/fs/reiserfs/fix_node.c
index 6c13a8d9a73c..3cbc8e4491ee 100644
--- a/fs/reiserfs/fix_node.c
+++ b/fs/reiserfs/fix_node.c
@@ -1926,6 +1926,7 @@ static int dc_check_balance_leaf(struct tree_balance *tb, int h)
{
struct virtual_node *vn = tb->tb_vn;
+#ifdef CONFIG_REISERFS_CHECK
/*
* Number of bytes that must be deleted from
* (value is negative if bytes are deleted) buffer which
@@ -1935,26 +1936,39 @@ static int dc_check_balance_leaf(struct tree_balance *tb, int h)
int levbytes;
/* the maximal item size */
- int maxsize, ret;
+ int maxsize;
+#endif
/*
* S0 is the node whose balance is currently being checked,
* and F0 is its father.
*/
- struct buffer_head *S0, *F0;
+
+#ifdef CONFIG_REISERFS_CHECK
+ struct buffer_head *S0;
+#endif
+ struct buffer_head *F0;
+
int lfree, rfree /* free space in L and R */ ;
+ int ret;
+#ifdef CONFIG_REISERFS_CHECK
S0 = PATH_H_PBUFFER(tb->tb_path, 0);
+#endif
F0 = PATH_H_PPARENT(tb->tb_path, 0);
+#ifdef CONFIG_REISERFS_CHECK
levbytes = tb->insert_size[h];
maxsize = MAX_CHILD_SIZE(S0); /* maximal possible size of an item */
+#endif
if (!F0) { /* S[0] is the root now. */
+#ifdef CONFIG_REISERFS_CHECK
RFALSE(-levbytes >= maxsize - B_FREE_SPACE(S0),
"vs-8240: attempt to create empty buffer tree");
+#endif
set_parameters(tb, h, 0, 0, 1, NULL, -1, -1);
return NO_BALANCING_NEEDED;
diff --git a/fs/reiserfs/lbalance.c b/fs/reiserfs/lbalance.c
index 7f868569d4d0..6f431819fd5e 100644
--- a/fs/reiserfs/lbalance.c
+++ b/fs/reiserfs/lbalance.c
@@ -521,8 +521,9 @@ static void leaf_item_bottle(struct buffer_info *dest_bi,
static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
int last_first, int cpy_num, int cpy_bytes)
{
- struct buffer_head *dest;
int pos, i, src_nr_item, bytes;
+#ifdef CONFIG_REISERFS_CHECK
+ struct buffer_head *dest;
dest = dest_bi->bi_bh;
RFALSE(!dest || !src, "vs-10210: !dest || !src");
@@ -532,6 +533,7 @@ static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
"vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
cpy_num);
RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
+#endif
if (cpy_num == 0)
return 0;
--
Best regards,
Mirsad Todorovac
> Honza
>
>
>> CC fs/reiserfs/lbalance.o
>> fs/reiserfs/fix_node.c: In function ‘dc_check_balance_leaf’:
>> fs/reiserfs/fix_node.c:1938:13: error: variable ‘maxsize’ set but not used [-Werror=unused-but-set-variable]
>> 1938 | int maxsize, ret;
>> | ^~~~~~~
>> fs/reiserfs/fix_node.c:1935:13: error: variable ‘levbytes’ set but not used [-Werror=unused-but-set-variable]
>> 1935 | int levbytes;
>> | ^~~~~~~~
>> fs/reiserfs/prints.c: In function ‘prepare_error_buf’:
>> fs/reiserfs/prints.c:221:17: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
>> 221 | p += vscnprintf(p, end - p, fmt1, args);
>> | ^
>> fs/reiserfs/prints.c:260:9: error: function ‘prepare_error_buf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
>> 260 | p += vscnprintf(p, end - p, fmt1, args);
>> | ^
>> make[4]: Target 'arch/x86/kernel/' not remade because of errors.
>> make[3]: *** [scripts/Makefile.build:485: arch/x86/kernel] Error 2
>> make[3]: Target 'arch/x86/' not remade because of errors.
>> make[2]: *** [scripts/Makefile.build:485: arch/x86] Error 2
>> fs/reiserfs/lbalance.c: In function ‘leaf_copy_items’:
>> fs/reiserfs/lbalance.c:524:29: error: variable ‘dest’ set but not used [-Werror=unused-but-set-variable]
>> 524 | struct buffer_head *dest;
>> | ^~~~
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/do_balan.o] Error 1
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/prints.o] Error 1
>> cc1: all warnings being treated as errors
>> make[4]: *** [scripts/Makefile.build:244: fs/reiserfs/fix_node.o] Error 1
>> ---------------------------------------------------------------------------------------------------------
>>
>> In fs/reiserfs/fix_node.c:1938:13, fs/reiserfs/fix_node.c:1935:13, and fs/reiserfs/lbalance.c:524:29,
>> the problem seem to lie within the construct RFALSE(), like
>>
>> 521 static int leaf_copy_items(struct buffer_info *dest_bi, struct buffer_head *src,
>> 522 int last_first, int cpy_num, int cpy_bytes)
>> 523 {
>> 524 struct buffer_head *dest;
>> 525 int pos, i, src_nr_item, bytes;
>> 526
>> 527 dest = dest_bi->bi_bh;
>> 528 RFALSE(!dest || !src, "vs-10210: !dest || !src");
>> 529 RFALSE(last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST,
>> 530 "vs-10220:last_first != FIRST_TO_LAST && last_first != LAST_TO_FIRST");
>> 531 RFALSE(B_NR_ITEMS(src) < cpy_num,
>> 532 "vs-10230: No enough items: %d, req. %d", B_NR_ITEMS(src),
>> 533 cpy_num);
>> 534 RFALSE(cpy_num < 0, "vs-10240: cpy_num < 0 (%d)", cpy_num);
>>
>> Hope this helps.
>>
>> Best regards,
>> Mirsad Todorovac
>>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-16 17:17 ` Mirsad Todorovac
@ 2024-07-17 15:44 ` Jan Kara
2024-07-17 22:14 ` Mirsad Todorovac
0 siblings, 1 reply; 16+ messages in thread
From: Jan Kara @ 2024-07-17 15:44 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Jan Kara, Linux Kernel Mailing List, Gustavo A. R. Silva,
Kees Cook, Christian Brauner, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
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
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-17 15:44 ` Jan Kara
@ 2024-07-17 22:14 ` Mirsad Todorovac
2024-07-18 9:39 ` Jan Kara
0 siblings, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-07-17 22:14 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-17 22:14 ` Mirsad Todorovac
@ 2024-07-18 9:39 ` Jan Kara
2024-07-19 20:51 ` Mirsad Todorovac
2024-08-02 16:31 ` Mirsad Todorovac
0 siblings, 2 replies; 16+ messages in thread
From: Jan Kara @ 2024-07-18 9:39 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Jan Kara, Linux Kernel Mailing List, Gustavo A. R. Silva,
Kees Cook, Christian Brauner, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
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
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-18 9:39 ` Jan Kara
@ 2024-07-19 20:51 ` Mirsad Todorovac
2024-08-02 16:31 ` Mirsad Todorovac
1 sibling, 0 replies; 16+ messages in thread
From: Mirsad Todorovac @ 2024-07-19 20:51 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-07-18 9:39 ` Jan Kara
2024-07-19 20:51 ` Mirsad Todorovac
@ 2024-08-02 16:31 ` Mirsad Todorovac
2024-08-05 13:04 ` Jan Kara
1 sibling, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-08-02 16:31 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
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:
-------------------><---------------------------------------
diff --git a/fs/reiserfs/do_balan.c b/fs/reiserfs/do_balan.c
index 5129efc6f2e6..c8fa3d71ef63 100644
--- a/fs/reiserfs/do_balan.c
+++ b/fs/reiserfs/do_balan.c
@@ -81,11 +81,11 @@ static void balance_leaf_when_delete_del(struct tree_balance *tb)
struct buffer_info bi;
#ifdef CONFIG_REISERFS_CHECK
struct item_head *ih = item_head(tbS0, item_pos);
-#endif
RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
"vs-12013: mode Delete, insert size %d, ih to be deleted %h",
-tb->insert_size[0], ih);
+#endif
buffer_info_init_tbS0(tb, &bi);
leaf_delete_items(&bi, 0, item_pos, 1, -1);
@@ -1797,8 +1797,8 @@ static inline void do_balance_starts(struct tree_balance *tb)
print_tb(flag, PATH_LAST_POSITION(tb->tb_path),
tb->tb_path->pos_in_item, tb, "check");
*/
- RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
#ifdef CONFIG_REISERFS_CHECK
+ RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
REISERFS_SB(tb->tb_sb)->cur_tb = tb;
#endif
}
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index f0e1f29f20ee..027e64853710 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -916,7 +916,7 @@ do { \
#if defined( CONFIG_REISERFS_CHECK )
#define RFALSE(cond, format, args...) __RASSERT(!(cond), "!(" #cond ")", format, ##args)
#else
-#define RFALSE( cond, format, args... ) do {;} while( 0 )
+#define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
#endif
#define CONSTF __attribute_const__
diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
index 5faf702f8d15..eed1a461169e 100644
--- a/fs/reiserfs/stree.c
+++ b/fs/reiserfs/stree.c
@@ -1280,7 +1280,9 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th,
&del_size,
max_reiserfs_offset(inode));
+#ifdef CONFIG_REISERFS_CHECK
RFALSE(mode != M_DELETE, "PAP-5320: mode must be M_DELETE");
+#endif
copy_item_head(&s_ih, tp_item_head(path));
s_del_balance.insert_size[0] = del_size;
--
Thanks.
Best regards,
Mirsad Todorovac
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-02 16:31 ` Mirsad Todorovac
@ 2024-08-05 13:04 ` Jan Kara
2024-08-05 21:24 ` Mirsad Todorovac
0 siblings, 1 reply; 16+ messages in thread
From: Jan Kara @ 2024-08-05 13:04 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Jan Kara, Linux Kernel Mailing List, Gustavo A. R. Silva,
Kees Cook, Christian Brauner, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
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
>
> -------------------><---------------------------------------
> diff --git a/fs/reiserfs/do_balan.c b/fs/reiserfs/do_balan.c
> index 5129efc6f2e6..c8fa3d71ef63 100644
> --- a/fs/reiserfs/do_balan.c
> +++ b/fs/reiserfs/do_balan.c
> @@ -81,11 +81,11 @@ static void balance_leaf_when_delete_del(struct tree_balance *tb)
> struct buffer_info bi;
> #ifdef CONFIG_REISERFS_CHECK
> struct item_head *ih = item_head(tbS0, item_pos);
> -#endif
>
> RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
> "vs-12013: mode Delete, insert size %d, ih to be deleted %h",
> -tb->insert_size[0], ih);
> +#endif
>
> buffer_info_init_tbS0(tb, &bi);
> leaf_delete_items(&bi, 0, item_pos, 1, -1);
> @@ -1797,8 +1797,8 @@ static inline void do_balance_starts(struct tree_balance *tb)
> print_tb(flag, PATH_LAST_POSITION(tb->tb_path),
> tb->tb_path->pos_in_item, tb, "check");
> */
> - RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
> #ifdef CONFIG_REISERFS_CHECK
> + RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
> REISERFS_SB(tb->tb_sb)->cur_tb = tb;
> #endif
> }
> diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
> index f0e1f29f20ee..027e64853710 100644
> --- a/fs/reiserfs/reiserfs.h
> +++ b/fs/reiserfs/reiserfs.h
> @@ -916,7 +916,7 @@ do { \
> #if defined( CONFIG_REISERFS_CHECK )
> #define RFALSE(cond, format, args...) __RASSERT(!(cond), "!(" #cond ")", format, ##args)
> #else
> -#define RFALSE( cond, format, args... ) do {;} while( 0 )
> +#define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
> #endif
>
> #define CONSTF __attribute_const__
> diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
> index 5faf702f8d15..eed1a461169e 100644
> --- a/fs/reiserfs/stree.c
> +++ b/fs/reiserfs/stree.c
> @@ -1280,7 +1280,9 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th,
> &del_size,
> max_reiserfs_offset(inode));
>
> +#ifdef CONFIG_REISERFS_CHECK
> RFALSE(mode != M_DELETE, "PAP-5320: mode must be M_DELETE");
> +#endif
>
> copy_item_head(&s_ih, tp_item_head(path));
> s_del_balance.insert_size[0] = del_size;
> --
>
> Thanks.
>
> Best regards,
> Mirsad Todorovac
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-05 13:04 ` Jan Kara
@ 2024-08-05 21:24 ` Mirsad Todorovac
2024-08-06 8:25 ` Jan Kara
0 siblings, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-08-05 21:24 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
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?
Thank you very much.
Best regards,
Mirsad Todorovac
>> -------------------><---------------------------------------
>> diff --git a/fs/reiserfs/do_balan.c b/fs/reiserfs/do_balan.c
>> index 5129efc6f2e6..c8fa3d71ef63 100644
>> --- a/fs/reiserfs/do_balan.c
>> +++ b/fs/reiserfs/do_balan.c
>> @@ -81,11 +81,11 @@ static void balance_leaf_when_delete_del(struct tree_balance *tb)
>> struct buffer_info bi;
>> #ifdef CONFIG_REISERFS_CHECK
>> struct item_head *ih = item_head(tbS0, item_pos);
>> -#endif
>>
>> RFALSE(ih_item_len(ih) + IH_SIZE != -tb->insert_size[0],
>> "vs-12013: mode Delete, insert size %d, ih to be deleted %h",
>> -tb->insert_size[0], ih);
>> +#endif
>>
>> buffer_info_init_tbS0(tb, &bi);
>> leaf_delete_items(&bi, 0, item_pos, 1, -1);
>> @@ -1797,8 +1797,8 @@ static inline void do_balance_starts(struct tree_balance *tb)
>> print_tb(flag, PATH_LAST_POSITION(tb->tb_path),
>> tb->tb_path->pos_in_item, tb, "check");
>> */
>> - RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
>> #ifdef CONFIG_REISERFS_CHECK
>> + RFALSE(check_before_balancing(tb), "PAP-12340: locked buffers in TB");
>> REISERFS_SB(tb->tb_sb)->cur_tb = tb;
>> #endif
>> }
>> diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
>> index f0e1f29f20ee..027e64853710 100644
>> --- a/fs/reiserfs/reiserfs.h
>> +++ b/fs/reiserfs/reiserfs.h
>> @@ -916,7 +916,7 @@ do { \
>> #if defined( CONFIG_REISERFS_CHECK )
>> #define RFALSE(cond, format, args...) __RASSERT(!(cond), "!(" #cond ")", format, ##args)
>> #else
>> -#define RFALSE( cond, format, args... ) do {;} while( 0 )
>> +#define RFALSE( cond, format, args... ) do { (void) (cond); } while( 0 )
>> #endif
>>
>> #define CONSTF __attribute_const__
>> diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c
>> index 5faf702f8d15..eed1a461169e 100644
>> --- a/fs/reiserfs/stree.c
>> +++ b/fs/reiserfs/stree.c
>> @@ -1280,7 +1280,9 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th,
>> &del_size,
>> max_reiserfs_offset(inode));
>>
>> +#ifdef CONFIG_REISERFS_CHECK
>> RFALSE(mode != M_DELETE, "PAP-5320: mode must be M_DELETE");
>> +#endif
>>
>> copy_item_head(&s_ih, tp_item_head(path));
>> s_del_balance.insert_size[0] = del_size;
>> --
>>
>> Thanks.
>>
>> Best regards,
>> Mirsad Todorovac
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-05 21:24 ` Mirsad Todorovac
@ 2024-08-06 8:25 ` Jan Kara
2024-08-11 13:34 ` Mirsad Todorovac
0 siblings, 1 reply; 16+ messages in thread
From: Jan Kara @ 2024-08-06 8:25 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Jan Kara, Linux Kernel Mailing List, Gustavo A. R. Silva,
Kees Cook, Christian Brauner, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
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@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-06 8:25 ` Jan Kara
@ 2024-08-11 13:34 ` Mirsad Todorovac
2024-08-11 20:52 ` Mirsad Todorovac
0 siblings, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-08-11 13:34 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
On 8/6/24 10:25, Jan Kara wrote:
> 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
Aaah, sorry I've just noticed your reply. I missed it this morning already.
Yes, at your request, I will proceed as you recommended.
Best regards,
Mirsad Todorovac
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-11 13:34 ` Mirsad Todorovac
@ 2024-08-11 20:52 ` Mirsad Todorovac
2024-08-27 9:24 ` Jan Kara
0 siblings, 1 reply; 16+ messages in thread
From: Mirsad Todorovac @ 2024-08-11 20:52 UTC (permalink / raw)
To: Jan Kara
Cc: Linux Kernel Mailing List, Gustavo A. R. Silva, Kees Cook,
Christian Brauner, Matthew Wilcox (Oracle), Al Viro, Jeff Layton,
reiserfs-devel
On 8/11/24 15:34, Mirsad Todorovac wrote:
> On 8/6/24 10:25, Jan Kara wrote:
>> 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
>
> Aaah, sorry I've just noticed your reply. I missed it this morning already.
>
> Yes, at your request, I will proceed as you recommended.
Hi, Jan,
As the filesystem has problems with the "General Protection Fault", which I learned later and
I am really not qualified to deal with that, just fixing compiler warnings is indeed cosmetics and an
exercise for the little grey cells.
Es ist nicht meine ernst, as Germans would have said.
But I will trust your judgment on whether this is worth patching.
Best regards,
Mirsad Todorovac
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PROBLEM linux-next] fs/reiserfs/do_balan.c:1147:13: error: variable ‘leaf_mi’ set but not used [-Werror=unused-but-set-variable]
2024-08-11 20:52 ` Mirsad Todorovac
@ 2024-08-27 9:24 ` Jan Kara
0 siblings, 0 replies; 16+ messages in thread
From: Jan Kara @ 2024-08-27 9:24 UTC (permalink / raw)
To: Mirsad Todorovac
Cc: Jan Kara, Linux Kernel Mailing List, Gustavo A. R. Silva,
Kees Cook, Christian Brauner, Matthew Wilcox (Oracle), Al Viro,
Jeff Layton, reiserfs-devel
On Sun 11-08-24 22:52:03, Mirsad Todorovac wrote:
> On 8/11/24 15:34, Mirsad Todorovac wrote:
> > On 8/6/24 10:25, Jan Kara wrote:
> >> 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
> >
> > Aaah, sorry I've just noticed your reply. I missed it this morning already.
> >
> > Yes, at your request, I will proceed as you recommended.
>
> Hi, Jan,
>
> As the filesystem has problems with the "General Protection Fault", which I learned later and
> I am really not qualified to deal with that, just fixing compiler warnings is indeed cosmetics and an
> exercise for the little grey cells.
>
> Es ist nicht meine ernst, as Germans would have said.
>
> But I will trust your judgment on whether this is worth patching.
As I wrote earlier, I don't think fixing warning is reiserfs code is really
worth it. It is like polishing windows on a car you're going to send to
scrapyard because the engine is broken :). But Linux is a shared project so
I also don't want to block other people from doing things they find
sensible... That's why if you send me a proper patch for this, I'll just
merge it.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-08-27 9:24 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-10 18:09 [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
2024-07-10 18:17 ` Kees Cook
2024-07-10 19:14 ` Mirsad Todorovac
2024-07-15 17:28 ` Jan Kara
2024-07-16 17:17 ` Mirsad Todorovac
2024-07-17 15:44 ` Jan Kara
2024-07-17 22:14 ` Mirsad Todorovac
2024-07-18 9:39 ` Jan Kara
2024-07-19 20:51 ` Mirsad Todorovac
2024-08-02 16:31 ` Mirsad Todorovac
2024-08-05 13:04 ` Jan Kara
2024-08-05 21:24 ` Mirsad Todorovac
2024-08-06 8:25 ` Jan Kara
2024-08-11 13:34 ` Mirsad Todorovac
2024-08-11 20:52 ` Mirsad Todorovac
2024-08-27 9:24 ` Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.