Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform
Hi,
There are 7 sources still call basename() directly and block/vvfat.c
define its own static basename() function. Please see the grep below:
➜ qemu git:(patch-v4) ✗ grep "basename(" **/*.c | grep -v get_basename
fsdev/virtfs-proxy-helper.c:basename(prog));
hw/vfio/pci.c:group_name = basename(iommu_group_path);
hw/vfio/platform.c:group_name = basename(iommu_group_path);
linux-user/elfload.c:base_filename = strdup(basename(filename));
qemu-io.c:progname = basename(argv[0]);
qemu-nbd.c:snprintf(sockpath, 128, SOCKET_PATH, basename(device));
qga/commands-posix.c:driver = g_strdup(basename(buf));
qga/commands-posix.c:fs->name = g_strdup(basename(syspath));
➜ qemu git:(patch-v4) ✗ grep get_basename **/*.c
block/vvfat.c:static const char* get_basename(const char* path)
block/vvfat.c: basename2 = get_basename(path);
block/vvfat.c:basename = get_basename(mapping->path);
block/vvfat.c: const char* basename = get_basename(mapping->path);
block/vvfat.c: const char* basename = get_basename(mapping->path);
block/vvfat.c: const char* basename2 = get_basename(path);
block/vvfat.c: - strlen(get_basename(commit->path)) - 1;
linux-user/elfload.c:base_filename =
g_path_get_basename(ts->bprm->filename);
Directly change all of them to g_path_get_basename ?
2015-10-06 21:22 GMT+08:00 Paolo Bonzini :
>
>
> On 06/10/2015 14:13, Eric Blake wrote:
>>
>> If you are already writing code to be ported to both Unixy and
>> windows systems, you are better off rolling your own alternative to
>> basename (or better, using something else that has already rolled a
>> portable version for you - while I know gnulib does that, we aren't
>> using gnulib; but I assume glib has something along those lines
>> even though I haven't looked for it).
>
> Yes, there is g_path_get_basename (and g_path_get_dirname). Added to
> http://wiki.qemu.org/BiteSizedTasks#API_conversion.
>
> Paolo
--
Best regards,
Houcheng Lin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform
On Sat, Oct 03, 2015 at 12:44:14PM +0800, Houcheng Lin wrote:
> diff --git a/configure b/configure
> index d7c24cd..cda88c1 100755
> --- a/configure
> +++ b/configure
> @@ -567,7 +567,6 @@ fi
>
> # host *BSD for user mode
> HOST_VARIANT_DIR=""
> -
> case $targetos in
> CYGWIN*)
>mingw32="yes"
Spurious whitespace change
> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index b1beaa6..44beee3 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -22,7 +22,6 @@
> */
> #include
> #include
> -#include
> #include
> #include
> #include
What is the justification for this? Do you know why io.h was included
before?
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ab3c876..9e26d10 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -74,6 +74,14 @@ typedef unsigned intuint_fast16_t;
> typedef signed int int_fast16_t;
> #endif
>
> +#ifdef CONFIG_ANDROID
> +/*
> + * For include the basename prototyping in android.
> + */
> +#include
Files that use basename(3) should include libgen.h. Why include it
here?
> +#define IOV_MAX 1024
Are you sure that Android NDK headers do not contain this constant?
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 3ae4987..4ae746b 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -62,6 +62,8 @@ extern int daemon(int, int);
> #include
> #include
> #include
> +#include
Why did you include time.h?
> +#include
>
> #ifdef CONFIG_LINUX
> #include
> @@ -482,3 +484,17 @@ int qemu_read_password(char *buf, int buf_size)
> printf("\n");
> return ret;
> }
> +
> +int qemu_getdtablesize(void)
> +{
> +#ifdef CONFIG_ANDROID
> +struct rlimit r;
> +
> +if (getrlimit(RLIMIT_NOFILE, ) < 0) {
> +return sysconf(_SC_OPEN_MAX);
> +}
> +return r.rlim_cur;
> +#else
> +return getdtablesize();
> +#endif
> +}
We can probably drop the getdtablesize() call completely and use the
CONFIG_ANDROID code on all platforms. I suggest splitting this out into
a separate patch that introduces qemu_getdtablesize() and converts all
callers.
> diff --git a/util/qemu-openpty.c b/util/qemu-openpty.c
> index 4c53211..b305886 100644
> --- a/util/qemu-openpty.c
> +++ b/util/qemu-openpty.c
> @@ -51,12 +51,17 @@
> # include
> #endif
>
> -#ifdef __sun__
> +#if defined(__sun__) || defined(CONFIG_ANDROID)
> +
> /* Once Solaris has openpty(), this is going to be removed. */
> static int openpty(int *amaster, int *aslave, char *name,
> struct termios *termp, struct winsize *winp)
> {
> +#if defined(CONFIG_ANDROID)
> +char slave[PATH_MAX];
> +#else
> const char *slave;
> +#endif
> int mfd = -1, sfd = -1;
>
> *amaster = *aslave = -1;
> @@ -67,17 +72,22 @@ static int openpty(int *amaster, int *aslave, char *name,
>
> if (grantpt(mfd) == -1 || unlockpt(mfd) == -1)
> goto err;
> -
> +#if defined(CONFIG_ANDROID)
> +if (ptsname_r(mfd, slave, PATH_MAX) < 0)
> +goto err;
> +#else
> if ((slave = ptsname(mfd)) == NULL)
> goto err;
> +#endif
ptsname_r(3) should be used on all Linux hosts because it is reentrant.
This improvement isn't Android-specific, please split it into a separate
patch.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform
Just a couple comments since I reviewed the previous versions... On 06/10/2015 11:47, Stefan Hajnoczi wrote: > > #include > > -#include > > #include > > #include > > #include > > What is the justification for this? Do you know why io.h was included > before? No reason, the same patch is en route through qemu-trivial. >> >> - >> +#if defined(CONFIG_ANDROID) >> +if (ptsname_r(mfd, slave, PATH_MAX) < 0) >> +goto err; >> +#else >> if ((slave = ptsname(mfd)) == NULL) >> goto err; >> +#endif > > ptsname_r(3) should be used on all Linux hosts because it is reentrant. > This improvement isn't Android-specific, please split it into a separate > patch. Actually everyone except Solaris and Android is already using openpty. This is emulation code for those two OSes. (The gnulib manual mentions that AIX 5.1, HP-UX 11, IRIX 6.5 also don't have openpty, but we don't support those I think). Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform
On 10/06/2015 03:47 AM, Stefan Hajnoczi wrote: > On Sat, Oct 03, 2015 at 12:44:14PM +0800, Houcheng Lin wrote: >> diff --git a/configure b/configure >> index d7c24cd..cda88c1 100755 >> +++ b/include/qemu/osdep.h >> @@ -74,6 +74,14 @@ typedef unsigned intuint_fast16_t; >> typedef signed int int_fast16_t; >> #endif >> >> +#ifdef CONFIG_ANDROID >> +/* >> + * For include the basename prototyping in android. >> + */ >> +#include > > Files that use basename(3) should include libgen.h. Why include it > here? What is using basename(3)? POSIX gives basename(3) a worthless contract - it is free to return non-thread-safe storage. Also, it is not portable to Windows-style drive-letters, so I consider any code using to already be suspect. If you are already writing code to be ported to both Unixy and windows systems, you are better off rolling your own alternative to basename (or better, using something else that has already rolled a portable version for you - while I know gnulib does that, we aren't using gnulib; but I assume glib has something along those lines even though I haven't looked for it). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform
On 06/10/2015 14:13, Eric Blake wrote: > > If you are already writing code to be ported to both Unixy and > windows systems, you are better off rolling your own alternative to > basename (or better, using something else that has already rolled a > portable version for you - while I know gnulib does that, we aren't > using gnulib; but I assume glib has something along those lines > even though I haven't looked for it). Yes, there is g_path_get_basename (and g_path_get_dirname). Added to http://wiki.qemu.org/BiteSizedTasks#API_conversion. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
