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@xxxxxxxx> wrote: > > Shyam Prasad N <nspmangalore@xxxxxxxxx> 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
Attachment:
0001-mount.cifs-use-SUDO_UID-env-variable-for-cruid.patch
Description: Binary data
