On Wed, 23 Mar 2011 13:02:51 -0500 Steve French <smfrench@xxxxxxxxx> wrote: > On Wed, Mar 23, 2011 at 12:58 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > 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. > > We could make it a union presumably, but it means more > casts as the sizes of fields are different, and smb2 doesn't > need some of cifs fields, and smb2 will need at least 5 > (but probably 10) more fields such as for "async response > received" and for chains of commands. Overall that will > cause the size of the structure for cifs to grow at least > 25% (probably more). > > Maybe you misunderstand me. The struct is *exactly the same* aside from a few small differences. It doesn't even need to be a union, IMO. That would let you use a lot of the same code in transport.c for smb2. -- 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