[PATCH] Add close_fn in fruit and streams_xattr module

[Jeremy Allison]

On Wed, Dec 19, 2018 at 05:46:47PM +0100, Günther Deschner via samba-technical wrote:
> Hi,
> 
> attached two fixes to add close_fns to fruit and streams_xattr, noticed
> while adding fruit on top of gluster.  More patches will follow to fully
> deal with local access to the AppleDouble files.
> 
> Please review and push,
> Guenther
> -- 
> Günther Deschner                    GPG-ID: 8EE11688
> Red Hat                         gdeschner at redhat.com
> Samba Team                              gd at samba.org

I don't think the first patch is *quite* right (close though).

In the open path we have:

       if (is_afpinfo_stream(smb_fname)) {
                fd = fruit_open_meta(handle, smb_fname, fsp, flags, mode);
        } else if (is_afpresource_stream(smb_fname)) {
                fd = fruit_open_rsrc(handle, smb_fname, fsp, flags, mode);
        } else {
                fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode);
        }

fruit_open_meta() can call either fruit_open_meta_stream()
or fruit_open_meta_netatalk() depending on config.

       switch (config->meta) {
        case FRUIT_META_STREAM:
                fd = fruit_open_meta_stream(handle, smb_fname,
                                            fsp, flags, mode);
                break;

        case FRUIT_META_NETATALK:
                fd = fruit_open_meta_netatalk(handle, smb_fname,
                                              fsp, flags, mode);
                break;

The close() call below is correct for fruit_open_meta_netatalk(),
as that opens the fruit_fake_fd() pipes, but in fruit_open_meta_stream()
it calls:

static int fruit_open_meta_stream(vfs_handle_struct *handle,
                                  struct smb_filename *smb_fname,
                                  files_struct *fsp,
                                  int flags,
                                  mode_t mode)
{
        struct fruit_config_data *config = NULL;
        struct fio *fio = NULL;
        int open_flags = flags & ~O_CREAT;
        int fd;

        DBG_DEBUG("Path [%s]\n", smb_fname_str_dbg(smb_fname));

        SMB_VFS_HANDLE_GET_DATA(handle, config,
                                struct fruit_config_data, return -1);

        fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL);
        fio->type = ADOUBLE_META;
        fio->config = config;

        fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, open_flags, mode);
        if (fd != -1) {
                return fd;
        }

So I think below you need another function below the
if (is_afpinfo_stream(fsp->fsp_name)) check that
only calls close() in the config->meta==FRUIT_META_NETATALK
case, but calls SMB_VFS_NEXT_CLOSE() in the
config->meta==FRUIT_META_STREAM case.

> +static int fruit_close(vfs_handle_struct *handle,
> +                       files_struct *fsp)
> +{
> +     int ret;
> +     int fd;
> +
> +     fd = fsp->fh->fd;
> +
> +     DBG_DEBUG("Path [%s] fd [%d]\n", smb_fname_str_dbg(fsp->fsp_name), fd);
> +
> +     if (!is_ntfs_stream_smb_fname(fsp->fsp_name)) {
> +             return SMB_VFS_NEXT_CLOSE(handle, fsp);
> +     }
> +
> +     if (is_afpinfo_stream(fsp->fsp_name)) {
> +             ret = close(fd);

                ^^^^^^^^^^^^^^^^
                This needs to check config->meta
                and disambiguate I think.

> +             fsp->fh->fd = -1;
> +     } else if (is_afpresource_stream(fsp->fsp_name)) {
> +             ret = fruit_close_rsrc(handle, fsp);
> +     } else {
> +             ret = SMB_VFS_NEXT_CLOSE(handle, fsp);
> +     }
> +
> +     return ret;
> +}

The streams_xattr patch looks spot on though.
That one, RB+ me.

Cheers,

	Jeremy.