From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjOq8-00027L-3J for qemu-devel@nongnu.org; Tue, 06 Oct 2015 05:47:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjOq4-0000Kc-Sv for qemu-devel@nongnu.org; Tue, 06 Oct 2015 05:47:20 -0400 Received: from mail-wi0-x233.google.com ([2a00:1450:400c:c05::233]:37417) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjOq4-0000KY-K7 for qemu-devel@nongnu.org; Tue, 06 Oct 2015 05:47:16 -0400 Received: by wicfx3 with SMTP id fx3so150523815wic.0 for ; Tue, 06 Oct 2015 02:47:16 -0700 (PDT) Date: Tue, 6 Oct 2015 10:47:13 +0100 From: Stefan Hajnoczi Message-ID: <20151006094713.GD19089@stefanha-thinkpad> References: <1443847454-20406-1-git-send-email-houcheng@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1443847454-20406-1-git-send-email-houcheng@gmail.com> Subject: Re: [Qemu-devel] [RFC PATCH v4] os-android: Add support to android platform List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Houcheng Lin Cc: peter.maydell@linaro.org, famz@redhat.com, kvm@vger.kernel.org, linhaocheng@itri.org.tw, qemu-devel@nongnu.org, pbonzini@redhat.com, alex.bennee@linaro.org 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 int uint_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, &r) < 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.