| Message ID | 17b8b06c00e359d1cb71b883371ad8c677a0e477.1527310210.git.keno@alumni.harvard.edu |
|---|---|
| State | New |
| Headers | show |
| Series | 9p: Add support for Darwin | expand |
On Sat, 26 May 2018 01:23:08 -0400 keno@juliacomputing.com wrote: > From: Keno Fischer <keno@alumni.harvard.edu> > > - Darwin doesn't have strchrnul > - Comparisons of mode_t with -1 require an explicit cast, since mode_t > is unsigned on Darwin. > > Signed-off-by: Keno Fischer <keno@juliacomputing.com> > --- > hw/9pfs/9p-local.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c > index fd65d04..6e0b2e8 100644 > --- a/hw/9pfs/9p-local.c > +++ b/hw/9pfs/9p-local.c > @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, > assert(*path != '/'); > > head = g_strdup(path); > - c = strchrnul(path, '/'); > + /* equivalent to strchrnul(), but that is not available on Darwin */ Please make a qemu_strchrnul() helper with a separate implementation for Darwin then. I guess you can put it in this file since there aren't any other users in the QEMU code base. > + c = strchr(path, '/'); > + if (!c) > + c = path + strlen(path); > if (*c) { > /* Intermediate path element */ > head[c - path] = 0; > @@ -310,7 +313,7 @@ update_map_file: > if (credp->fc_gid != -1) { > gid = credp->fc_gid; > } > - if (credp->fc_mode != -1) { > + if (credp->fc_mode != (mode_t)-1) { > mode = credp->fc_mode; > } > if (credp->fc_rdev != -1) { > @@ -416,7 +419,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp) > return err; > } > } > - if (credp->fc_mode != -1) { > + if (credp->fc_mode != (mode_t)-1) { > uint32_t tmp_mode = cpu_to_le32(credp->fc_mode); > err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.mode", &tmp_mode, > sizeof(mode_t), 0);
>> --- a/hw/9pfs/9p-local.c >> +++ b/hw/9pfs/9p-local.c >> @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, >> assert(*path != '/'); >> >> head = g_strdup(path); >> - c = strchrnul(path, '/'); >> + /* equivalent to strchrnul(), but that is not available on Darwin */ > > Please make a qemu_strchrnul() helper with a separate implementation for Darwin > then. I guess you can put it in this file since there aren't any other users in > the QEMU code base. There actually are, but they also use this pattern. Could you suggest the best place to put this utility? I can submit a patch to switch all instances of this pattern over.
On Thu, 31 May 2018 12:27:35 -0400 Keno Fischer <keno@juliacomputing.com> wrote: > >> --- a/hw/9pfs/9p-local.c > >> +++ b/hw/9pfs/9p-local.c > >> @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, > >> assert(*path != '/'); > >> > >> head = g_strdup(path); > >> - c = strchrnul(path, '/'); > >> + /* equivalent to strchrnul(), but that is not available on Darwin */ > > > > Please make a qemu_strchrnul() helper with a separate implementation for Darwin > > then. I guess you can put it in this file since there aren't any other users in > > the QEMU code base. > > There actually are, but they also use this pattern. Could you > suggest the best place to put this utility? I can submit a patch > to switch all instances of this pattern over. Oh if the pattern is already used in other places, it's probably not worth the pain... so please forget this :)
Well, I do have the patch already to switch this and the other patterns, so let me know if you want it or not ;). On Thu, May 31, 2018 at 3:22 PM, Greg Kurz <groug@kaod.org> wrote: > On Thu, 31 May 2018 12:27:35 -0400 > Keno Fischer <keno@juliacomputing.com> wrote: > >> >> --- a/hw/9pfs/9p-local.c >> >> +++ b/hw/9pfs/9p-local.c >> >> @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, >> >> assert(*path != '/'); >> >> >> >> head = g_strdup(path); >> >> - c = strchrnul(path, '/'); >> >> + /* equivalent to strchrnul(), but that is not available on Darwin */ >> > >> > Please make a qemu_strchrnul() helper with a separate implementation for Darwin >> > then. I guess you can put it in this file since there aren't any other users in >> > the QEMU code base. >> >> There actually are, but they also use this pattern. Could you >> suggest the best place to put this utility? I can submit a patch >> to switch all instances of this pattern over. > > Oh if the pattern is already used in other places, it's probably not > worth the pain... so please forget this :)
On Thu, 31 May 2018 15:23:45 -0400 Keno Fischer <keno@juliacomputing.com> wrote: > Well, I do have the patch already to switch this and the other patterns, > so let me know if you want it or not ;). > Post it then and we'll see if people are happy with that :) > On Thu, May 31, 2018 at 3:22 PM, Greg Kurz <groug@kaod.org> wrote: > > On Thu, 31 May 2018 12:27:35 -0400 > > Keno Fischer <keno@juliacomputing.com> wrote: > > > >> >> --- a/hw/9pfs/9p-local.c > >> >> +++ b/hw/9pfs/9p-local.c > >> >> @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, > >> >> assert(*path != '/'); > >> >> > >> >> head = g_strdup(path); > >> >> - c = strchrnul(path, '/'); > >> >> + /* equivalent to strchrnul(), but that is not available on Darwin */ > >> > > >> > Please make a qemu_strchrnul() helper with a separate implementation for Darwin > >> > then. I guess you can put it in this file since there aren't any other users in > >> > the QEMU code base. > >> > >> There actually are, but they also use this pattern. Could you > >> suggest the best place to put this utility? I can submit a patch > >> to switch all instances of this pattern over. > > > > Oh if the pattern is already used in other places, it's probably not > > worth the pain... so please forget this :)
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c index fd65d04..6e0b2e8 100644 --- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -67,7 +67,10 @@ int local_open_nofollow(FsContext *fs_ctx, const char *path, int flags, assert(*path != '/'); head = g_strdup(path); - c = strchrnul(path, '/'); + /* equivalent to strchrnul(), but that is not available on Darwin */ + c = strchr(path, '/'); + if (!c) + c = path + strlen(path); if (*c) { /* Intermediate path element */ head[c - path] = 0; @@ -310,7 +313,7 @@ update_map_file: if (credp->fc_gid != -1) { gid = credp->fc_gid; } - if (credp->fc_mode != -1) { + if (credp->fc_mode != (mode_t)-1) { mode = credp->fc_mode; } if (credp->fc_rdev != -1) { @@ -416,7 +419,7 @@ static int local_set_xattrat(int dirfd, const char *path, FsCred *credp) return err; } } - if (credp->fc_mode != -1) { + if (credp->fc_mode != (mode_t)-1) { uint32_t tmp_mode = cpu_to_le32(credp->fc_mode); err = fsetxattrat_nofollow(dirfd, path, "user.virtfs.mode", &tmp_mode, sizeof(mode_t), 0);