On Thu, 2014-05-01 at 10:31 -0500, Shirish Pargaonkar wrote: > > + > > +out: > > + sess_data->result = rc; > > + sess_data->func = NULL; > > + sess_free_buffer(sess_data); > > + sess_establish_session(sess_data); > > + kfree(ses->auth_key.response); > > + ses->auth_key.response = NULL; > > +} > > In all three sess_auth_* functions (ntlm, ntlmv2, and lanman (in > previous patch), > we will end up calling sess_establish_session() even if there is an error i.e. > sess_data->result is not 0. We check for any errors within sess_establish_session() before we attempt to establish session. static int sess_establish_session(struct sess_data *sess_data) { .. if (!sess_data->result) { //Establish Session. } .. } This isn't a technical issue but maybe considered a style issue. > > + if (phase == NtLmChallenge) { > > + rc = decode_ntlmssp_challenge(bcc_ptr, blob_len, ses); > > + /* now goto beginning for ntlmssp authenticate phase */ > > I think this comment should move to after if statement > (it was at wrong place to begin with) i.e. go to authenticate phase > only if rc is NULL. I agree. The comment is actually not required. Sachin Prabhu -- 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