This is useful list. I will look through your questions, and reposting comments to linux-cifs. On Wed, Mar 23, 2011 at 10:58 AM, Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: > Steve, > > Some review comments (on smb2misc.c, will review the rest). > > > smb2misc.c > > 1. I think we should change check_smb2_hdr to look like check_smb_hdr. > ProtocolId and MessageId are compared twice. yes. agreed. > 2. The mid is declared as __u16 in function check_smb2_hdr but in > function checkSMB2, it is declared as __u64. Good catch. > 3. In function checkSMB2, we probably do not need the first else. > 4. In function checkSMB2, perhaps we should use CIFS_MAX_MSGSIZE instead > of variable CIFSMaxBufSize. This is the similar to cifs (eventually we can change the pool size and design for SMB2, since optimal (and maximum possible) sizes for SMB2 requests are far larger than for cifs but this is reasonable for initial default for large buffers (affects readdir responses mainly, since Jeremy had done a readpages that uses an array of pages rather than large buffers for SMB2, since it is impractical or impossible to allocate slab buffers large enough to hold smb2's large read responses). The comparison is: len < (CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4) simply because that is the size of the objects in the pool. > 4. Do we need to apply le64_to_cpu to smb->MessageId while comparing > to mid? No. The MessageId is opaque (same as in cifs for the Mid, albeit for SMB2 much larger and doesn't wrap). -- Thanks, Steve -- 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