Re: [Qemu-devel] [PATCH 05/13] 9p: darwin: Handle struct dirent differences

[Greg Kurz]

On Sat, 26 May 2018 01:23:07 -0400
address@hidden wrote:

> From: Keno Fischer <address@hidden>
> 
> Signed-off-by: Keno Fischer <address@hidden>
> ---
>  hw/9pfs/9p-synth.c |  4 ++++
>  hw/9pfs/9p.c       | 18 ++++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index eb68b42..3c0a6d8 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -221,7 +221,11 @@ static void synth_direntry(V9fsSynthNode *node,
>  {
>      strcpy(entry->d_name, node->name);
>      entry->d_ino = node->attr->inode;
> +#ifdef CONFIG_DARWIN
> +    entry->d_seekoff = off + 1;

Hmm... what's that for ? No users in the patchset and the comment
below seem to indicate this wouldn't be trusted anyway.

> +#else
>      entry->d_off = off + 1;
> +#endif
>  }
>  
>  static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 1cfdf7d..49654ae 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1801,7 +1801,11 @@ static int coroutine_fn 
> v9fs_do_readdir_with_stat(V9fsPDU *pdu,
>          count += len;
>          v9fs_stat_free(&v9stat);
>          v9fs_path_free(&path);
> +#ifdef CONFIG_DARWIN
> +        saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> +#else
>          saved_dir_pos = dent->d_off;
> +#endif
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -1952,9 +1956,19 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, 
> V9fsFidState *fidp,
>          qid.type = 0;
>          qid.version = 0;
>  
> +#ifdef CONFIG_DARWIN
> +        // Darwin has d_seekoff, which appears to function
> +        // analogously to d_off. However, it does not appear
> +        // to be supported on all file systems, so use
> +        // telldir for correctness.

Please use /* */

> +        off_t off = v9fs_co_telldir(pdu, fidp);

Please declare local variables at the beginning of the function. Also,
v9fs_co_telldir() can fail. This requires proper error handling.

> +#else
> +        off_t off = dent->d_off;
> +#endif

Please make this a helper and call it in v9fs_do_readdir_with_stat() as well.

> +
>          /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
>          len = pdu_marshal(pdu, 11 + count, "Qqbs",
> -                          &qid, dent->d_off,
> +                          &qid, off,
>                            dent->d_type, &name);
>  
>          v9fs_readdir_unlock(&fidp->fs.dir);
> @@ -1966,7 +1980,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, 
> V9fsFidState *fidp,
>          }
>          count += len;
>          v9fs_string_free(&name);
> -        saved_dir_pos = dent->d_off;
> +        saved_dir_pos = off;
>      }
>  
>      v9fs_readdir_unlock(&fidp->fs.dir);