linux-kernel - Re: [PATCH 15/79] bfs: switch to new ctime accessors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <646b7283ede4945b335ad16aea5ff60e1361241e.camel@kernel.org>
Date:   Wed, 21 Jun 2023 12:57:19 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     Christian Brauner <brauner@...nel.org>,
        "Tigran A. Aivazian" <aivazian.tigran@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 15/79] bfs: switch to new ctime accessors

On Wed, 2023-06-21 at 18:48 +0200, Jan Kara wrote:
> On Wed 21-06-23 10:45:28, Jeff Layton wrote:
> > In later patches, we're going to change how the ctime.tv_nsec field is
> > utilized. Switch to using accessor functions instead of raw accesses of
> > inode->i_ctime.
> > 
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> 
> ...
> 
> > diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
> > index 1926bec2c850..c964316be32b 100644
> > --- a/fs/bfs/inode.c
> > +++ b/fs/bfs/inode.c
> > @@ -82,10 +82,10 @@ struct inode *bfs_iget(struct super_block *sb, unsigned long ino)
> >  	inode->i_blocks = BFS_FILEBLOCKS(di);
> >  	inode->i_atime.tv_sec =  le32_to_cpu(di->i_atime);
> >  	inode->i_mtime.tv_sec =  le32_to_cpu(di->i_mtime);
> > -	inode->i_ctime.tv_sec =  le32_to_cpu(di->i_ctime);
> > +	inode_ctime_set_sec(inode, le32_to_cpu(di->i_ctime));
> >  	inode->i_atime.tv_nsec = 0;
> >  	inode->i_mtime.tv_nsec = 0;
> > -	inode->i_ctime.tv_nsec = 0;
> > +	inode_ctime_set_nsec(inode, 0);
> 
> So I'm somewhat wondering here - in other filesystem you construct
> timespec64 and then use inode_ctime_set(). Here you use
> inode_ctime_set_sec() + inode_ctime_set_nsec(). What's the benefit? It
> seems these two functions are not used that much some maybe we could just
> live with just inode_ctime_set() and constructing timespec64 when needed?
> 
> 								Honza

The main advantage is that by using that, I didn't need to do quite so
much of this conversion by hand. My coccinelle skills are pretty
primitive. I went with whatever conversion was going to give minimal
changes, to the existing accesses for the most part.

We could certainly do it the way you suggest, it just means having to
re-touch a lot of this code by hand, or someone with better coccinelle
chops suggesting a way to declare a temporary variables in place.
-- 
Jeff Layton <jlayton@...nel.org>