From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
linux-hardening@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH][next] f2fs: Replace one-element array with flexible-array member
Date: Sat, 27 Feb 2021 21:00:51 -0800 [thread overview]
Message-ID: <YDsjg1LqnkYIvvtB@google.com> (raw)
In-Reply-To: <bee16b72-f2e2-b113-9785-7f760be867df@huawei.com>
On 02/25, Chao Yu wrote:
> Hello, Gustavo,
>
> On 2021/2/25 3:03, Gustavo A. R. Silva wrote:
> > There is a regular need in the kernel to provide a way to declare having
> > a dynamically sized set of trailing elements in a structure. Kernel code
> > should always use “flexible array members”[1] for these cases. The older
> > style of one-element or zero-length arrays should no longer be used[2].
>
> I proposal to do the similar cleanup, and I've no objection on doing this.
>
> https://lore.kernel.org/patchwork/patch/869440/
>
> Let's ask for Jaegeuk' opinion.
Merged, thanks.
This looks better reason than code readability. :)
>
> >
> > Refactor the code according to the use of a flexible-array member in
> > struct f2fs_checkpoint, instead of a one-element arrays.
> >
> > Notice that a temporary pointer to void '*tmp_ptr' was used in order to
> > fix the following errors when using a flexible array instead of a one
> > element array in struct f2fs_checkpoint:
> >
> > CC [M] fs/f2fs/dir.o
> > In file included from fs/f2fs/dir.c:13:
> > fs/f2fs/f2fs.h: In function ‘__bitmap_ptr’:
> > fs/f2fs/f2fs.h:2227:40: error: invalid use of flexible array member
> > 2227 | return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > | ^
> > fs/f2fs/f2fs.h:2227:49: error: invalid use of flexible array member
> > 2227 | return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > | ^
> > fs/f2fs/f2fs.h:2238:40: error: invalid use of flexible array member
> > 2238 | return &ckpt->sit_nat_version_bitmap + offset;
> > | ^
> > make[2]: *** [scripts/Makefile.build:287: fs/f2fs/dir.o] Error 1
> > make[1]: *** [scripts/Makefile.build:530: fs/f2fs] Error 2
> > make: *** [Makefile:1819: fs] Error 2
> >
> > [1] https://en.wikipedia.org/wiki/Flexible_array_member
> > [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Link: https://github.com/KSPP/linux/issues/79
> > Build-tested-by: kernel test robot <lkp@intel.com>
> > Link: https://lore.kernel.org/lkml/603647e4.DeEFbl4eqljuwAUe%25lkp@intel.com/
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > fs/f2fs/f2fs.h | 5 +++--
> > include/linux/f2fs_fs.h | 2 +-
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index e2d302ae3a46..3f5cb097c30f 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -2215,6 +2215,7 @@ static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
> > static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > {
> > struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> > + void *tmp_ptr = &ckpt->sit_nat_version_bitmap;
> > int offset;
> > if (is_set_ckpt_flags(sbi, CP_LARGE_NAT_BITMAP_FLAG)) {
> > @@ -2224,7 +2225,7 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > * if large_nat_bitmap feature is enabled, leave checksum
> > * protection for all nat/sit bitmaps.
> > */
> > - return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > + return tmp_ptr + offset + sizeof(__le32);
> > }
> > if (__cp_payload(sbi) > 0) {
> > @@ -2235,7 +2236,7 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > } else {
> > offset = (flag == NAT_BITMAP) ?
> > le32_to_cpu(ckpt->sit_ver_bitmap_bytesize) : 0;
> > - return &ckpt->sit_nat_version_bitmap + offset;
> > + return tmp_ptr + offset;
> > }
> > }
> > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > index c6cc0a566ef5..5487a80617a3 100644
> > --- a/include/linux/f2fs_fs.h
> > +++ b/include/linux/f2fs_fs.h
> > @@ -168,7 +168,7 @@ struct f2fs_checkpoint {
> > unsigned char alloc_type[MAX_ACTIVE_LOGS];
> > /* SIT and NAT version bitmap */
> > - unsigned char sit_nat_version_bitmap[1];
> > + unsigned char sit_nat_version_bitmap[];
> > } __packed;
> > #define CP_CHKSUM_OFFSET 4092 /* default chksum offset in checkpoint */
> >
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
Chao Yu <chao@kernel.org>,
linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH][next] f2fs: Replace one-element array with flexible-array member
Date: Sat, 27 Feb 2021 21:00:51 -0800 [thread overview]
Message-ID: <YDsjg1LqnkYIvvtB@google.com> (raw)
In-Reply-To: <bee16b72-f2e2-b113-9785-7f760be867df@huawei.com>
On 02/25, Chao Yu wrote:
> Hello, Gustavo,
>
> On 2021/2/25 3:03, Gustavo A. R. Silva wrote:
> > There is a regular need in the kernel to provide a way to declare having
> > a dynamically sized set of trailing elements in a structure. Kernel code
> > should always use “flexible array members”[1] for these cases. The older
> > style of one-element or zero-length arrays should no longer be used[2].
>
> I proposal to do the similar cleanup, and I've no objection on doing this.
>
> https://lore.kernel.org/patchwork/patch/869440/
>
> Let's ask for Jaegeuk' opinion.
Merged, thanks.
This looks better reason than code readability. :)
>
> >
> > Refactor the code according to the use of a flexible-array member in
> > struct f2fs_checkpoint, instead of a one-element arrays.
> >
> > Notice that a temporary pointer to void '*tmp_ptr' was used in order to
> > fix the following errors when using a flexible array instead of a one
> > element array in struct f2fs_checkpoint:
> >
> > CC [M] fs/f2fs/dir.o
> > In file included from fs/f2fs/dir.c:13:
> > fs/f2fs/f2fs.h: In function ‘__bitmap_ptr’:
> > fs/f2fs/f2fs.h:2227:40: error: invalid use of flexible array member
> > 2227 | return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > | ^
> > fs/f2fs/f2fs.h:2227:49: error: invalid use of flexible array member
> > 2227 | return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > | ^
> > fs/f2fs/f2fs.h:2238:40: error: invalid use of flexible array member
> > 2238 | return &ckpt->sit_nat_version_bitmap + offset;
> > | ^
> > make[2]: *** [scripts/Makefile.build:287: fs/f2fs/dir.o] Error 1
> > make[1]: *** [scripts/Makefile.build:530: fs/f2fs] Error 2
> > make: *** [Makefile:1819: fs] Error 2
> >
> > [1] https://en.wikipedia.org/wiki/Flexible_array_member
> > [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> >
> > Link: https://github.com/KSPP/linux/issues/79
> > Build-tested-by: kernel test robot <lkp@intel.com>
> > Link: https://lore.kernel.org/lkml/603647e4.DeEFbl4eqljuwAUe%25lkp@intel.com/
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> > fs/f2fs/f2fs.h | 5 +++--
> > include/linux/f2fs_fs.h | 2 +-
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index e2d302ae3a46..3f5cb097c30f 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -2215,6 +2215,7 @@ static inline block_t __cp_payload(struct f2fs_sb_info *sbi)
> > static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > {
> > struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
> > + void *tmp_ptr = &ckpt->sit_nat_version_bitmap;
> > int offset;
> > if (is_set_ckpt_flags(sbi, CP_LARGE_NAT_BITMAP_FLAG)) {
> > @@ -2224,7 +2225,7 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > * if large_nat_bitmap feature is enabled, leave checksum
> > * protection for all nat/sit bitmaps.
> > */
> > - return &ckpt->sit_nat_version_bitmap + offset + sizeof(__le32);
> > + return tmp_ptr + offset + sizeof(__le32);
> > }
> > if (__cp_payload(sbi) > 0) {
> > @@ -2235,7 +2236,7 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info *sbi, int flag)
> > } else {
> > offset = (flag == NAT_BITMAP) ?
> > le32_to_cpu(ckpt->sit_ver_bitmap_bytesize) : 0;
> > - return &ckpt->sit_nat_version_bitmap + offset;
> > + return tmp_ptr + offset;
> > }
> > }
> > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > index c6cc0a566ef5..5487a80617a3 100644
> > --- a/include/linux/f2fs_fs.h
> > +++ b/include/linux/f2fs_fs.h
> > @@ -168,7 +168,7 @@ struct f2fs_checkpoint {
> > unsigned char alloc_type[MAX_ACTIVE_LOGS];
> > /* SIT and NAT version bitmap */
> > - unsigned char sit_nat_version_bitmap[1];
> > + unsigned char sit_nat_version_bitmap[];
> > } __packed;
> > #define CP_CHKSUM_OFFSET 4092 /* default chksum offset in checkpoint */
> >
next prev parent reply other threads:[~2021-02-28 5:01 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-24 19:03 [f2fs-dev] [PATCH][next] f2fs: Replace one-element array with flexible-array member Gustavo A. R. Silva
2021-02-24 19:03 ` Gustavo A. R. Silva
2021-02-25 1:37 ` [f2fs-dev] " Chao Yu
2021-02-25 1:37 ` Chao Yu
2021-02-28 5:00 ` Jaegeuk Kim [this message]
2021-02-28 5:00 ` Jaegeuk Kim
2021-03-01 3:17 ` Chao Yu
2021-03-01 3:17 ` Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YDsjg1LqnkYIvvtB@google.com \
--to=jaegeuk@kernel.org \
--cc=gustavoars@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.