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.
>
>
>> > + 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
