[PATCH] Samba: CephFS Snapshots VFS module

David Disseldorp ddiss at suse.de
Mon May 13 10:27:38 UTC 2019


On Fri, 10 May 2019 11:58:41 -0700, Jeremy Allison wrote:

> Can you change the comment to be:
> 
> +       /*
> +        * found snapshot via parent. Append the child path component
> +        * that was trimmed... +1 for path separator + 1 for null termination.
> +        */
> +       if (strlen(_converted_buf) + 1 + strlen(trimmed) + 1 > buflen) {
> +               return -EINVAL;
> +       }
> 
> Just to use the expected idion of '>' rather than the rarer
> '>=' when checking string overruns.  
> 
> So the result would be:
> 
> +       /*
> +        * found snapshot via parent. Append the child path component
> +        * that was trimmed... +1 for path separator + 1 for null termination.
> +        */
> +       if (strlen(_converted_buf) + 1 + strlen(trimmed) + 1 > buflen) {
> +               return -EINVAL;
> +       }
> +       strlcat(_converted_buf, "/", buflen);
> +       strlcat(_converted_buf, trimmed, buflen);

Changed.

> Second comment - in ceph_snap_gmt_opendir() you do:
> 
> +       dir = SMB_VFS_NEXT_OPENDIR(handle, conv_smb_fname, mask, attr);
> +       saved_errno = errno;
> +       TALLOC_FREE(conv_smb_fname);
> +       errno = saved_errno;
> +       return dir;
> 
> - NB, you're saving errno and restoring over the TALLOC_FREE(conv_smb_fname);
> I think that's the right thing to do (you never know
> if TALLOC_FREE might do a syscall to overwrite errno).

I really wish we didn't use errno across the VFS interface :-)
New saved_errno version attached...

Cheers, David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vfs_ceph_snapshots_v3.patchset
Type: text/x-patch
Size: 57231 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190513/1b0b64f2/vfs_ceph_snapshots_v3.bin>


More information about the samba-technical mailing list