| Message ID | 20200615040141.3627746-1-yanaijie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | erofs: Eliminate usage of uninitialized_var() macro | expand |
Hi Jason, On Mon, Jun 15, 2020 at 12:01:41PM +0800, Jason Yan wrote: > This is an effort to eliminate the uninitialized_var() macro[1]. > > The use of this macro is the wrong solution because it forces off ANY > analysis by the compiler for a given variable. It even masks "unused > variable" warnings. > > Quoted from Linus[2]: > > "It's a horrible thing to use, in that it adds extra cruft to the > source code, and then shuts up a compiler warning (even the _reliable_ > warnings from gcc)." > > The gcc option "-Wmaybe-uninitialized" has been disabled and this change > will not produce any warnnings even with "make W=1". > > [1] https://github.com/KSPP/linux/issues/81 > [2] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/ > > Cc: Kees Cook <keescook@chromium.org> > Cc: Chao Yu <yuchao0@huawei.com> > Signed-off-by: Jason Yan <yanaijie@huawei.com> > --- I'm fine with the patch since "-Wmaybe-uninitialized" has been disabled and I've also asked Kees for it in private previously. I still remembered that Kees sent out a treewide patch. Sorry about that I don't catch up it... But what is wrong with the original patchset? Thanks, Gao Xiang
在 2020/6/15 15:25, Gao Xiang 写道: > Hi Jason, > > On Mon, Jun 15, 2020 at 12:01:41PM +0800, Jason Yan wrote: >> This is an effort to eliminate the uninitialized_var() macro[1]. >> >> The use of this macro is the wrong solution because it forces off ANY >> analysis by the compiler for a given variable. It even masks "unused >> variable" warnings. >> >> Quoted from Linus[2]: >> >> "It's a horrible thing to use, in that it adds extra cruft to the >> source code, and then shuts up a compiler warning (even the _reliable_ >> warnings from gcc)." >> >> The gcc option "-Wmaybe-uninitialized" has been disabled and this change >> will not produce any warnnings even with "make W=1". >> >> [1] https://github.com/KSPP/linux/issues/81 >> [2] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/ >> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: Chao Yu <yuchao0@huawei.com> >> Signed-off-by: Jason Yan <yanaijie@huawei.com> >> --- > > I'm fine with the patch since "-Wmaybe-uninitialized" has been disabled and > I've also asked Kees for it in private previously. > > I still remembered that Kees sent out a treewide patch. Sorry about that > I don't catch up it... But what is wrong with the original patchset? > Yes, Kees has remind me of that and I will let him handle it. So you can ignore this patch. Thanks, Jason > Thanks, > Gao Xiang > > > . >
On 2020/6/15 16:07, Gao Xiang wrote: > On Mon, Jun 15, 2020 at 03:43:09PM +0800, Jason Yan wrote: >> >> >> 鍦?2020/6/15 15:25, Gao Xiang 鍐欓亾: >>> Hi Jason, >>> >>> On Mon, Jun 15, 2020 at 12:01:41PM +0800, Jason Yan wrote: >>>> This is an effort to eliminate the uninitialized_var() macro[1]. >>>> >>>> The use of this macro is the wrong solution because it forces off ANY >>>> analysis by the compiler for a given variable. It even masks "unused >>>> variable" warnings. >>>> >>>> Quoted from Linus[2]: >>>> >>>> "It's a horrible thing to use, in that it adds extra cruft to the >>>> source code, and then shuts up a compiler warning (even the _reliable_ >>>> warnings from gcc)." >>>> >>>> The gcc option "-Wmaybe-uninitialized" has been disabled and this change >>>> will not produce any warnnings even with "make W=1". >>>> >>>> [1] https://github.com/KSPP/linux/issues/81 >>>> [2] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/ >>>> >>>> Cc: Kees Cook <keescook@chromium.org> >>>> Cc: Chao Yu <yuchao0@huawei.com> >>>> Signed-off-by: Jason Yan <yanaijie@huawei.com> >>>> --- >>> >>> I'm fine with the patch since "-Wmaybe-uninitialized" has been disabled and >>> I've also asked Kees for it in private previously. >>> >>> I still remembered that Kees sent out a treewide patch. Sorry about that >>> I don't catch up it... But what is wrong with the original patchset? >>> >> >> Yes, Kees has remind me of that and I will let him handle it. So you can >> ignore this patch. > > Okay, I was just wondering if this part should be send out via EROFS tree > for this cycle. However if there was an automatic generated patch by Kees, > I think perhaps Linus could pick them out directly. But anyway, both ways > are fine with me. ;) Ping me when needed. Either way is okay to me. Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > > Thanks, > Gao Xiang > >> >> Thanks, >> Jason >> >>> Thanks, >>> Gao Xiang >>> >>> >>> . >>> >> > > . >
diff --git a/fs/erofs/data.c b/fs/erofs/data.c index 64b56c7df023..d0542151e8c4 100644 --- a/fs/erofs/data.c +++ b/fs/erofs/data.c @@ -265,7 +265,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio, */ static int erofs_raw_access_readpage(struct file *file, struct page *page) { - erofs_off_t uninitialized_var(last_block); + erofs_off_t last_block; struct bio *bio; trace_erofs_readpage(page, true); @@ -282,7 +282,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page) static void erofs_raw_access_readahead(struct readahead_control *rac) { - erofs_off_t uninitialized_var(last_block); + erofs_off_t last_block; struct bio *bio = NULL; struct page *page; diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c index be50a4d9d273..24a26aaf847f 100644 --- a/fs/erofs/zdata.c +++ b/fs/erofs/zdata.c @@ -1161,7 +1161,7 @@ static void z_erofs_submit_queue(struct super_block *sb, struct z_erofs_decompressqueue *q[NR_JOBQUEUES]; void *bi_private; /* since bio will be NULL, no need to initialize last_index */ - pgoff_t uninitialized_var(last_index); + pgoff_t last_index; unsigned int nr_bios = 0; struct bio *bio = NULL;
This is an effort to eliminate the uninitialized_var() macro[1]. The use of this macro is the wrong solution because it forces off ANY analysis by the compiler for a given variable. It even masks "unused variable" warnings. Quoted from Linus[2]: "It's a horrible thing to use, in that it adds extra cruft to the source code, and then shuts up a compiler warning (even the _reliable_ warnings from gcc)." The gcc option "-Wmaybe-uninitialized" has been disabled and this change will not produce any warnnings even with "make W=1". [1] https://github.com/KSPP/linux/issues/81 [2] https://lore.kernel.org/lkml/CA+55aFz2500WfbKXAx8s67wrm9=yVJu65TpLgN_ybYNv0VEOKA@mail.gmail.com/ Cc: Kees Cook <keescook@chromium.org> Cc: Chao Yu <yuchao0@huawei.com> Signed-off-by: Jason Yan <yanaijie@huawei.com> --- fs/erofs/data.c | 4 ++-- fs/erofs/zdata.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)