On Wed, 23 Mar 2011 13:38:38 -0500 Steve French <smfrench@xxxxxxxxx> wrote: > On Wed, Mar 23, 2011 at 1:32 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Wed, 23 Mar 2011 13:26:19 -0500 > > Steve French <smfrench@xxxxxxxxx> wrote: > > > >> On Wed, Mar 23, 2011 at 1:06 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:> > >> > 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. > >> > >> I don't mind trying to condense it to one structure, but it is not that simple: > >> - smb2 doesn't need or sequence number, and the two transact2 > >> multiresponse booleans > >> - cifs mid will grow by at least 10 bytes by expanding the mid, pid > >> etc. to the required minimum sizes. > >> - cifs mid doesn't need "async_response_received" or (that is an smb2 > >> only feature and that or equivalent is required). ÂWhen we add chained > >> commands (common in smb2) then complex_mid or equivalent as well as > >> the command list, the number of the commands received and the last > >> response time of entries will be useless to cifs. > >> > > > > Is it better to add an entirely new set of functions simply because > > you'll have some unused fields in this structure in either case? > > For allocating/freeing a mid I don't think it matters - it affects a > small number of functions, and will likely diverge more over time (as > smb2 is evolving with new dialects, while cifs, the protocol, is > static) - but I don't feel strongly about it. I generally would > prefer changing the minimal amount of cifs code, but if we want to > move to one function for both types of mids, then I would prefer a > union so it is harder to accidently use the cifs specific fields in > smb2 and vice versa. > > > It does matter. It's more code to maintain and you need an entirely different set of functions to deal with the fact that the struct is different. The function in $SUBJECT is a case in point. There'd be no need for it if this was a common struct shared between the two. Personally, my preference is for the least amount of specialized code. Disruption to the existing codebase needs to be carefully managed, but duplicating code like this ought to be a last resort. -- 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
