Re: [PATCH] sync_smb2_mid_result

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 23 Mar 2011 12:42:26 -0500
Steve French <smfrench@xxxxxxxxx> wrote:

> On Wed, Mar 23, 2011 at 11:06 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Sat, 12 Mar 2011 19:55:55 -0600
> > Steve French <smfrench@xxxxxxxxx> wrote:
> >
> >> Previously CIFS added a function, sync_mid_result, to better handle
> >> certain error conditions.
> >> This patch adds the equivalent for SMB2 mids (they are used by smb2_sendrcv2
> >> which is added in the next patch)
> 
> > Now that you've commented out all of the unneeded fields from the
> > struct smb2_mid_entry, it's clear that the smb2_mid_entry is a subset
> > of mid_q_entry. Some of the mid_q_entry fields might need to be
> > expanded (the mid number itself for example), but that hardly seems to
> > warrant a completely new struct and codebase for this.
> >
> > Can you explain why we need all of this code duplication when it now
> > seems fairly clear that much of the existing code could be repurposed
> > for SMB2? This approach is going to vastly increase the maintenance
> > burden and I don't see that we gain anything from it.
> 
> Do you mean changing the cifs mid from its current single struct to a
> nested mid_common which can be included in cifs and smb2 specific mids
> (or made into a union)
> 

No. I mean use the same struct for both, expanding fields in the "old"
mid_q_entry struct as needed. For the "mid" field itself, for instance
you could grow it to a u64 and just plan to not use the upper bits for
smb1. Other fields could be handed similarly.

A common struct here means that you can reuse more code.

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux