[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/9pfs: Add CephFS support in VirtFS
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH] hw/9pfs: Add CephFS support in VirtFS |
Date: |
Thu, 10 Mar 2016 10:08:13 +0100 |
On Wed, 9 Mar 2016 13:09:58 -0700
Eric Blake <address@hidden> wrote:
> On 03/09/2016 12:02 PM, Greg Kurz wrote:
> > On Wed, 2 Mar 2016 23:41:43 +0800
> > Jevon Qiao <address@hidden> wrote:
> >
>
> >> +}
> >> +
> >> +static int cephfs_readdir_r(FsContext *ctx, V9fsFidOpenState *fs,
> >> + struct dirent *entry,
> >> + struct dirent **result)
> >> +{
> >> + int ret;
>
> >> +
> >> + return ret;
> >
> > This function should behave like the original readdir_r() function from the
> > C library, but it doesn't.
> >
>
> readdir_r() is hopelessly broken. POSIX is withdrawing it as such.
> http://austingroupbugs.net/view.php?id=696
>
> readdir() should be all the more any sane program needs, because it
> should already be thread-safe.
>
I wasn't aware that readdir_r() was so badly broken. It is currently
used here:
hw/9pfs/9p-handle.c: return readdir_r(fs->dir, entry, result);
hw/9pfs/9p-local.c: ret = readdir_r(fs->dir, entry, result);
hw/9pfs/9p-proxy.c: return readdir_r(fs->dir, entry, result);
I'll see how we can move to readdir().
> > According to the the libcephfs.h header:
> >
> > * @returns 1 if the next entry was filled in, 0 if the end of the
> > directory stream was reached,
> > * and a negative error code on failure.
> > */
> > int ceph_readdir_r(struct ceph_mount_info *cmount, struct ceph_dir_result
> > *dirp, struct dirent *de);
> >
> > and the readdir_r() manual page says:
> >
> > The readdir_r() function returns 0 on success. On error, it returns a
> > positive error number (listed under ERRORS). If the end of the direcā
> > tory stream is reached, readdir_r() returns 0, and returns NULL in
> > *result.
>
> readdir_r() can silently overflow buffers, with no recourse. Its use
> should not be encouraged.
>
Sure... this being said, fsdev currently exposes a readdir_r operation, not
a readdir one. Until we decide to change that, backends must follow the
readdir_r() API.
Cheers.
--
Greg