Re: [PATCH RESEND] cifs: file: initialize oparms.reconnect beforeusing it

From: Andi Shyti
Date: Mon Jul 29 2013 - 12:34:18 EST


> > + oparms.tcon = tcon;
> > + oparms.cifs_sb = cifs_sb;
> > + oparms.desired_access = desired_access;
> > + oparms.create_options = create_options;
>
> This patch just moves the brokenness around. You're
> setting .desired_access here to an unintialized variable.
> create_options also looks like it may potentially be wrong at this
> point.

Urrrca! This is what I achieve when I do one last fix before
going to sleep.

I spent a bit of time more going through the cifs/smb code and
the most sensful fix looks this

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 1e57f36..7e36ae3 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -647,6 +647,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flu
oflags, &oplock, &cfile->fid.netfid, xid
if (rc == 0) {
cifs_dbg(FYI, "posix reopen succeeded\n");
+ oparms.reconnect = true;
goto reopen_success;
}
/*

There is only one case when reconnect becames false, that is when
open = smb2_open_file and calls SMB2_open() that calls
add_durable_context() that sets reconnect = false with some
nested ifs in between, and still only in case
everything succeeds. We are checking reconnect only for this
case, otherwise we could get rid of the if (oparms.reconnect) and
not falling into the unknown state.

If it makes sense, I can send the above suggestion.

Thanks,
Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/