So, the buildbot rightly complained that this can return an
uninitialized error variable in xfs_attr_shortform_addname now
(why are we disabling the goddam use of uninitialized variable
warnings in Linux again, sigh..).
I then not only created the trivial fix, but also wrote a simple wrapper
for the setxattr syscall as the existing setfattr and attr tools don't
allow to control the flag, which I assumed means xfstests didn't really
test this as much as it should. But that little test showed we're still
getting the right errno values even with the unininitialized variable
returns, which seemed odd.
It turns out we're not even exercising this code any more, as
xfs_attr_set already does a xfs_attr_lookup lookup first and has a
copy of this logic executed much earlier (and I should have really though
about that because I got very close to that code for the defer ops
cleanup).
So.. I'm tempted to just turn these checks into asserts with something
like the below on top of this patch, I'll just need to see if it survives
testing:
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d6173888ed0d56..abdc58f286154a 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1066,13 +1066,13 @@ xfs_attr_shortform_addname(
struct xfs_da_args *args)
{
int newsize, forkoff;
- int error;
trace_xfs_attr_sf_addname(args);
if (xfs_attr_sf_findname(args)) {
- if (!(args->op_flags & XFS_DA_OP_REPLACE))
- return error;
+ int error;
+
+ ASSERT(args->op_flags & XFS_DA_OP_REPLACE);
error = xfs_attr_sf_removename(args);
if (error)
@@ -1086,8 +1086,7 @@ xfs_attr_shortform_addname(
*/
args->op_flags &= ~XFS_DA_OP_REPLACE;
} else {
- if (args->op_flags & XFS_DA_OP_REPLACE)
- return error;
+ ASSERT(!(args->op_flags & XFS_DA_OP_REPLACE));
}
if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
