Re: [PATCH][next] fs: Fix uninitialized scalar variable now

From: Shuah Khan
Date: Wed Oct 09 2024 - 16:38:22 EST


On 10/9/24 14:05, Everest K.C. wrote:
Variable `now` is declared without initialization. The variable
could be accessed inside the if-else statements following the
variable declaration, before it has been initialized.

It could be, but it isn't. I am not sure if this change is needed.

This patch initializes the variable to
`inode_set_ctime_current(inode)` by default.

Instead of "This patch initializes", change it to "Initialize ..."
Do refer to submitting patches document for information on how
to write change logs.


This issue was reported by Coverity Scan.

Include the the error/report from Coverity.


Signed-off-by: Everest K.C. <everestkc@xxxxxxxxxxxxxxxx>
---
fs/attr.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index c614b954bda5..77523af2e62d 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL(inode_newsize_ok);
static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
{
unsigned int ia_valid = attr->ia_valid;
- struct timespec64 now;
+ struct timespec64 now = inode_set_ctime_current(inode);
if (ia_valid & ATTR_CTIME) {
/*
@@ -293,8 +293,6 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
*/
if (ia_valid & ATTR_DELEG)
now = inode_set_ctime_deleg(inode, attr->ia_ctime);
- else
- now = inode_set_ctime_current(inode);

The code is clear and easy to read the way it is since it handles both cases
and does appropriate initialization.


} else {
/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
WARN_ON_ONCE(ia_valid & ATTR_MTIME);

I will leave it up to the maintainers to decide whether to take
this change or not.

thanks,
-- Shuah