Implementing SMB_VFS_FCNTL in Samba

[Ralph Boehme]

On 10/4/19 11:47 AM, Anoop C S wrote:
> On Fri, 2019-10-04 at 10:38 +0200, Ralph Boehme via samba-technical
> wrote:
>> On 10/3/19 3:20 PM, Anoop C S via samba-technical wrote:
>>> On Thu, 2019-10-03 at 18:43 +0530, Anoop C S via samba-technical
>>> wrote:
>>>> OK. This should be it. I hope attached patch covers the missing
>>>> part
>>>> where recent fcntl() commands are detected during configure. A
>>>> pipeline
>>>> has been completed successfully for the attached patches.
>>>>
>>>> https://gitlab.com/samba-team/devel/samba/pipelines/86263033
>>>>
>>>> Reviews are appreciated.
>>>
>>> Please ignore the previous version which had a typo in checking
>>> HAVE_XX_XX inside vfs_default. Attaching the patches after
>>> correction.
>>
>> nice addition, thanks!
>>
>> One nitpick and one general question.
>>
>> Please don't do function calls in if expressions:
>>
>> if ((val = SMB_VFS_FCNTL(fsp, F_GETFL, 0)) == -1) {
>>     return -1;
>> }
>>
>> Instead:
>>
>> val = SMB_VFS_FCNTL(fsp, F_GETFL, 0);
>> if (val == -1) {
>>     return -1;
>> }
> 
> Right. I was also skeptical in putting it that way(frankly speaking it
> is a copy of current set_blocking() function). :-)
> 
> Please see the new patch set attached.
> 
>> Then, I wonder why you make a copy of va_args in the time_audit and
>> full_audit VFS modules before calling the NEXT function. Can't we
>> just pass the va_list on to the NEXT function?
> 
> Two reasons:
> 
> * Both SMB_VFS_FCNTL and SMB_VFS_NEXT_FCNTL invoke same
> smb_vfs_call_fcntl() function which does a va_start() on the received
> variable arguments.

doh!, of course. Pushed.

Thanks!
-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46