From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> 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: Mon, 1 Mar 2021 11:17:07 +0800 [thread overview] Message-ID: <c8f5daa3-ec01-c6ba-7823-04c3650b689a@huawei.com> (raw) In-Reply-To: <YDsjg1LqnkYIvvtB@google.com> On 2021/2/28 13:00, Jaegeuk Kim wrote: > 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. :) Agreed. Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > >> >>> >>> 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 */ >>> > . >
WARNING: multiple messages have this Message-ID (diff)
From: Chao Yu <yuchao0@huawei.com> To: Jaegeuk Kim <jaegeuk@kernel.org> 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: Mon, 1 Mar 2021 11:17:07 +0800 [thread overview] Message-ID: <c8f5daa3-ec01-c6ba-7823-04c3650b689a@huawei.com> (raw) In-Reply-To: <YDsjg1LqnkYIvvtB@google.com> On 2021/2/28 13:00, Jaegeuk Kim wrote: > 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. :) Agreed. Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > >> >>> >>> 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
next prev parent reply other threads:[~2021-03-01 3:17 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-24 19:03 [PATCH][next] f2fs: Replace one-element array with flexible-array member Gustavo A. R. Silva 2021-02-24 19:03 ` [f2fs-dev] " Gustavo A. R. Silva 2021-02-25 1:37 ` Chao Yu 2021-02-25 1:37 ` Chao Yu 2021-02-28 5:00 ` Jaegeuk Kim 2021-02-28 5:00 ` Jaegeuk Kim 2021-03-01 3:17 ` Chao Yu [this message] 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=c8f5daa3-ec01-c6ba-7823-04c3650b689a@huawei.com \ --to=yuchao0@huawei.com \ --cc=chao@kernel.org \ --cc=gustavoars@kernel.org \ --cc=jaegeuk@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-hardening@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ /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: linkBe 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.