| Message ID | dc091588d770923c3afe779e1eb512724662db71.1678290988.git.sweettea-kernel@dorminy.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix compilation error on sparc/parisc | expand |
On 3/8/23 07:58, Sweet Tea Dorminy wrote: > Commit 1ec49744ba83 ("btrfs: turn on -Wmaybe-uninitialized") exposed > that on sparc and parisc, gcc is unaware that fscrypt_setup_filename() > only returns negative error values or 0. This ultimately results in a > maybe-uninitialized warning in btrfs_lookup_dentry(). > > Change to only return negative error values or 0 from > fscrypt_setup_filename() at the relevant callsite, and assert that no > positive error codes are returned (which would have wider implications > involving other users). > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/all/481b19b5-83a0-4793-b4fd-194ad7b978c3@roeck-us.net/ > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> > --- > fs/btrfs/inode.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 45102785c723..50178609f241 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -5409,8 +5409,13 @@ static int btrfs_inode_by_name(struct btrfs_inode *dir, struct dentry *dentry, > return -ENOMEM; > > ret = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name, 1, &fname); > - if (ret) > + if (ret < 0) > goto out; > + /* > + * fscrypt_setup_filename() should never return a positive value, but > + * gcc on sparc/parisc thinks it can, so assert that doesn't happen. > + */ Here is some food for thought: 1) Make sure that "location" is never initialized in btrfs_inode_by_name(), and that the functions returns a value >= 0. 2) compile. gcc never reports an error or warning (tested with 10.3, 11.3, and 12.2). On x86, gcc doesn't report a problem even if I replace btrfs_inode_by_name() with a simple "return 0;". I looks like there is something else going on which prevents gcc on architectures other than parisc and sparc from reporting uninitialized stack variables (or at least uninitialized structure elements), and I am not entirely sure if this should be blamed on the architecture. More specifically and importantly, it may not be a good idea to expect gcc to report potentially uninitialized variables on architectures other than parisc and sparc. Guenter > + ASSERT(ret == 0); > > /* This needs to handle no-key deletions later on */ >
On Wed, Mar 08, 2023 at 10:58:36AM -0500, Sweet Tea Dorminy wrote: > Commit 1ec49744ba83 ("btrfs: turn on -Wmaybe-uninitialized") exposed > that on sparc and parisc, gcc is unaware that fscrypt_setup_filename() > only returns negative error values or 0. This ultimately results in a > maybe-uninitialized warning in btrfs_lookup_dentry(). > > Change to only return negative error values or 0 from > fscrypt_setup_filename() at the relevant callsite, and assert that no > positive error codes are returned (which would have wider implications > involving other users). > > Reported-by: Guenter Roeck <linux@roeck-us.net> > Link: https://lore.kernel.org/all/481b19b5-83a0-4793-b4fd-194ad7b978c3@roeck-us.net/ > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> Added to misc-next, thanks.
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 45102785c723..50178609f241 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5409,8 +5409,13 @@ static int btrfs_inode_by_name(struct btrfs_inode *dir, struct dentry *dentry, return -ENOMEM; ret = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name, 1, &fname); - if (ret) + if (ret < 0) goto out; + /* + * fscrypt_setup_filename() should never return a positive value, but + * gcc on sparc/parisc thinks it can, so assert that doesn't happen. + */ + ASSERT(ret == 0); /* This needs to handle no-key deletions later on */
Commit 1ec49744ba83 ("btrfs: turn on -Wmaybe-uninitialized") exposed that on sparc and parisc, gcc is unaware that fscrypt_setup_filename() only returns negative error values or 0. This ultimately results in a maybe-uninitialized warning in btrfs_lookup_dentry(). Change to only return negative error values or 0 from fscrypt_setup_filename() at the relevant callsite, and assert that no positive error codes are returned (which would have wider implications involving other users). Reported-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/all/481b19b5-83a0-4793-b4fd-194ad7b978c3@roeck-us.net/ Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> --- fs/btrfs/inode.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)