On Thu, 2014-05-01 at 15:01 -0500, Shirish Pargaonkar wrote: > On Thu, May 1, 2014 at 11:26 AM, Sachin Prabhu <sprabhu@xxxxxxxxxx> wrote: > > 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. > > Sure, would have preferred to call the function before goto lables, but will > leave at that. > Having given it another thought, I agree with you. I've made the required modifications and tested the new patches. I'll send them to the list shortly. Sachin Prabhu > > > > > >> > + 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