Attaching the patch with the review comments incorporated.
Tested a few positive and negative test cases. Works as expected.

Regards,
Shyam

On Thu, Sep 17, 2020 at 2:27 PM Aurélien Aptel <aaptel@suse.com> wrote:
>
> Shyam Prasad N <nspmangalore@gmail.com> writes:
> >> This function later forks, so if you allocate before the fork, you need
> >> to free in parent and in the child.
> >
> > Good point.
> > I think I'm doing it right though. I allocate all that I need at the beginning.
> > And the function always terminates in mount_exit, both for parent and
> > child. That is where the allocations are freed.
>
> Ah yes ok
>
> > I know the child will have the options buffer unnecessarily allocated,
> > but isn't the code flow simpler this way?
> > Please let me know if you see an issue there.
>
> No it's fine I think
>
> > Good catch. This code existed before my changes, and I had noted this
> > bug. But forgot it during my changes. :)
> > I was actually confused if I should reset after the label, or before the goto.
> > After the label is an added overhead in the "happy" code path, so went
> > with this. But it does reduce the chances of missing out a reset.
> > For now I'll reset options before each goto mount_retry. Please let me
> > know if you feel the other approach is better.
>
> I think we can agree that mount.cifs is not performance critical code
> but that it should be safe so I think reset after the label is
> better. (To be honnest the whole function could use some refactoring and
> be split up probably, but that can be a patch for later on)
>
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)



-- 
-Shyam