* [PATCH 0/3] fs: dlm: Fix some issues related to a buffer in dlm_create_debug_file()
@ 2023-09-22 17:31 Christophe JAILLET
2023-09-22 17:31 ` [PATCH 1/3] fs: dlm: Simplify buffer size computation " Christophe JAILLET
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Christophe JAILLET @ 2023-09-22 17:31 UTC (permalink / raw)
Cc: linux-kernel, kernel-janitors, Christophe JAILLET
The goal of this serie is to remove a warning when building with W=1.
(details in patch 2)
Patch 1 is a preparation step. It is a no-op. The generated code is the same.
Patch 2 is the real fix.
Patch 3 is an additionnal clean-up.
Christophe JAILLET (3):
fs: dlm: Simplify buffer size computation in dlm_create_debug_file()
fs: dlm: Fix the size of a buffer in dlm_create_debug_file()
fs: dlm: Remove some useless memset()
fs/dlm/debug_fs.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/3] fs: dlm: Simplify buffer size computation in dlm_create_debug_file()
2023-09-22 17:31 [PATCH 0/3] fs: dlm: Fix some issues related to a buffer in dlm_create_debug_file() Christophe JAILLET
@ 2023-09-22 17:31 ` Christophe JAILLET
2023-09-22 17:31 ` [PATCH 2/3] fs: dlm: Fix the size of a buffer " Christophe JAILLET
2023-09-22 17:31 ` [PATCH 3/3] fs: dlm: Remove some useless memset() Christophe JAILLET
2 siblings, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2023-09-22 17:31 UTC (permalink / raw)
To: Christine Caulfield, David Teigland
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, gfs2
Use sizeof(name) instead of the equivalent, but hard coded,
DLM_LOCKSPACE_LEN + 8.
This is less verbose and more future proof.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
fs/dlm/debug_fs.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index 5aabcb6f0f15..e9726c6cbdf2 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -986,7 +986,7 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 2 */
memset(name, 0, sizeof(name));
- snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_locks", ls->ls_name);
+ snprintf(name, sizeof(name), "%s_locks", ls->ls_name);
ls->ls_debug_locks_dentry = debugfs_create_file(name,
0644,
@@ -997,7 +997,7 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 3 */
memset(name, 0, sizeof(name));
- snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_all", ls->ls_name);
+ snprintf(name, sizeof(name), "%s_all", ls->ls_name);
ls->ls_debug_all_dentry = debugfs_create_file(name,
S_IFREG | S_IRUGO,
@@ -1008,7 +1008,7 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 4 */
memset(name, 0, sizeof(name));
- snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_toss", ls->ls_name);
+ snprintf(name, sizeof(name), "%s_toss", ls->ls_name);
ls->ls_debug_toss_dentry = debugfs_create_file(name,
S_IFREG | S_IRUGO,
@@ -1017,7 +1017,7 @@ void dlm_create_debug_file(struct dlm_ls *ls)
&format4_fops);
memset(name, 0, sizeof(name));
- snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
+ snprintf(name, sizeof(name), "%s_waiters", ls->ls_name);
ls->ls_debug_waiters_dentry = debugfs_create_file(name,
0644,
@@ -1028,7 +1028,7 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 5 */
memset(name, 0, sizeof(name));
- snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name);
+ snprintf(name, sizeof(name), "%s_queued_asts", ls->ls_name);
ls->ls_debug_queued_asts_dentry = debugfs_create_file(name,
0644,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] fs: dlm: Fix the size of a buffer in dlm_create_debug_file()
2023-09-22 17:31 [PATCH 0/3] fs: dlm: Fix some issues related to a buffer in dlm_create_debug_file() Christophe JAILLET
2023-09-22 17:31 ` [PATCH 1/3] fs: dlm: Simplify buffer size computation " Christophe JAILLET
@ 2023-09-22 17:31 ` Christophe JAILLET
2023-09-22 20:03 ` Alexander Aring
2023-09-22 17:31 ` [PATCH 3/3] fs: dlm: Remove some useless memset() Christophe JAILLET
2 siblings, 1 reply; 5+ messages in thread
From: Christophe JAILLET @ 2023-09-22 17:31 UTC (permalink / raw)
To: Christine Caulfield, David Teigland, Alexander Aring
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, gfs2
8 is not the maximum size of the suffix used when creating debugfs files.
Let the compiler compute the correct size, and only give a hint about the
longest possible string that is used.
When building with W=1, this fixes the following warnings:
fs/dlm/debug_fs.c: In function ‘dlm_create_debug_file’:
fs/dlm/debug_fs.c:1020:58: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
| ^
fs/dlm/debug_fs.c:1020:9: note: ‘snprintf’ output between 9 and 73 bytes into a destination of size 72
1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/dlm/debug_fs.c:1031:50: error: ‘_queued_asts’ directive output may be truncated writing 12 bytes into a region of size between 8 and 72 [-Werror=format-truncation=]
1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name);
| ^~~~~~~~~~~~
fs/dlm/debug_fs.c:1031:9: note: ‘snprintf’ output between 13 and 77 bytes into a destination of size 72
1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fixes: 541adb0d4d10b ("fs: dlm: debugfs for queued callbacks")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
fs/dlm/debug_fs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index e9726c6cbdf2..c93359ceaae6 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -973,7 +973,8 @@ void dlm_delete_debug_comms_file(void *ctx)
void dlm_create_debug_file(struct dlm_ls *ls)
{
- char name[DLM_LOCKSPACE_LEN + 8];
+ /* Reserve enough space for the longest file name */
+ char name[DLM_LOCKSPACE_LEN + sizeof("_queued_asts")];
/* format 1 */
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] fs: dlm: Remove some useless memset()
2023-09-22 17:31 [PATCH 0/3] fs: dlm: Fix some issues related to a buffer in dlm_create_debug_file() Christophe JAILLET
2023-09-22 17:31 ` [PATCH 1/3] fs: dlm: Simplify buffer size computation " Christophe JAILLET
2023-09-22 17:31 ` [PATCH 2/3] fs: dlm: Fix the size of a buffer " Christophe JAILLET
@ 2023-09-22 17:31 ` Christophe JAILLET
2 siblings, 0 replies; 5+ messages in thread
From: Christophe JAILLET @ 2023-09-22 17:31 UTC (permalink / raw)
To: Christine Caulfield, David Teigland
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, gfs2
There is no need to clear the buffer used to build the file name.
snprintf() already guarantees that it is NULL terminated and such a
(useless) precaution was not done for the first string (i.e
ls_debug_rsb_dentry)
So, save a few LoC.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
fs/dlm/debug_fs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/fs/dlm/debug_fs.c b/fs/dlm/debug_fs.c
index c93359ceaae6..42f332f46359 100644
--- a/fs/dlm/debug_fs.c
+++ b/fs/dlm/debug_fs.c
@@ -986,7 +986,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 2 */
- memset(name, 0, sizeof(name));
snprintf(name, sizeof(name), "%s_locks", ls->ls_name);
ls->ls_debug_locks_dentry = debugfs_create_file(name,
@@ -997,7 +996,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 3 */
- memset(name, 0, sizeof(name));
snprintf(name, sizeof(name), "%s_all", ls->ls_name);
ls->ls_debug_all_dentry = debugfs_create_file(name,
@@ -1008,7 +1006,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 4 */
- memset(name, 0, sizeof(name));
snprintf(name, sizeof(name), "%s_toss", ls->ls_name);
ls->ls_debug_toss_dentry = debugfs_create_file(name,
@@ -1017,7 +1014,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)
ls,
&format4_fops);
- memset(name, 0, sizeof(name));
snprintf(name, sizeof(name), "%s_waiters", ls->ls_name);
ls->ls_debug_waiters_dentry = debugfs_create_file(name,
@@ -1028,7 +1024,6 @@ void dlm_create_debug_file(struct dlm_ls *ls)
/* format 5 */
- memset(name, 0, sizeof(name));
snprintf(name, sizeof(name), "%s_queued_asts", ls->ls_name);
ls->ls_debug_queued_asts_dentry = debugfs_create_file(name,
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] fs: dlm: Fix the size of a buffer in dlm_create_debug_file()
2023-09-22 17:31 ` [PATCH 2/3] fs: dlm: Fix the size of a buffer " Christophe JAILLET
@ 2023-09-22 20:03 ` Alexander Aring
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Aring @ 2023-09-22 20:03 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Christine Caulfield, David Teigland, linux-kernel, kernel-janitors, gfs2
Hi,
On Fri, Sep 22, 2023 at 1:38 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
>
> 8 is not the maximum size of the suffix used when creating debugfs files.
>
> Let the compiler compute the correct size, and only give a hint about the
> longest possible string that is used.
>
> When building with W=1, this fixes the following warnings:
>
> fs/dlm/debug_fs.c: In function ‘dlm_create_debug_file’:
> fs/dlm/debug_fs.c:1020:58: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
> 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
> | ^
> fs/dlm/debug_fs.c:1020:9: note: ‘snprintf’ output between 9 and 73 bytes into a destination of size 72
> 1020 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_waiters", ls->ls_name);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/dlm/debug_fs.c:1031:50: error: ‘_queued_asts’ directive output may be truncated writing 12 bytes into a region of size between 8 and 72 [-Werror=format-truncation=]
> 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name);
> | ^~~~~~~~~~~~
> fs/dlm/debug_fs.c:1031:9: note: ‘snprintf’ output between 13 and 77 bytes into a destination of size 72
> 1031 | snprintf(name, DLM_LOCKSPACE_LEN + 8, "%s_queued_asts", ls->ls_name);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fixes: 541adb0d4d10b ("fs: dlm: debugfs for queued callbacks")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
there is a similar patch [0] already in the queue for dlm.
- Alex
[0] https://lore.kernel.org/gfs2/20230906143153.1353077-5-aahringo@redhat.com/T/#u
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-22 20:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-22 17:31 [PATCH 0/3] fs: dlm: Fix some issues related to a buffer in dlm_create_debug_file() Christophe JAILLET
2023-09-22 17:31 ` [PATCH 1/3] fs: dlm: Simplify buffer size computation " Christophe JAILLET
2023-09-22 17:31 ` [PATCH 2/3] fs: dlm: Fix the size of a buffer " Christophe JAILLET
2023-09-22 20:03 ` Alexander Aring
2023-09-22 17:31 ` [PATCH 3/3] fs: dlm: Remove some useless memset() Christophe JAILLET
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.