From 6b3f49a5374305ce9690c3c5ca2aadc90f54c521 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 6 Mar 2013 16:20:55 -0800 Subject: [PATCH] Upgrade to current NetBSD popen/pclose. This gets us back to using vfork now our ARM vfork assembler stub is fixed, and adds the missing thread safety for the 'pidlist'. Bug: 5335385 Change-Id: Ib08bfa65b2cb9fa695717aae629ea14816bf988d --- libc/Android.mk | 2 +- libc/upstream-netbsd/env.h | 22 +++ .../libc/gen}/popen.c | 157 ++++++++++++------ libc/upstream-netbsd/netbsd-compat.h | 9 + libc/upstream-netbsd/reentrant.h | 15 +- tests/stdio_test.cpp | 12 ++ 6 files changed, 168 insertions(+), 49 deletions(-) create mode 100644 libc/upstream-netbsd/env.h rename libc/{unistd => upstream-netbsd/libc/gen}/popen.c (57%) diff --git a/libc/Android.mk b/libc/Android.mk index 4470efa86..b033790a0 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -11,7 +11,6 @@ libc_common_src_files := \ unistd/exec.c \ unistd/fnmatch.c \ unistd/getopt_long.c \ - unistd/popen.c \ unistd/syslog.c \ unistd/system.c \ unistd/time.c \ @@ -326,6 +325,7 @@ libc_upstream_netbsd_src_files := \ upstream-netbsd/libc/gen/ftw.c \ upstream-netbsd/libc/gen/nftw.c \ upstream-netbsd/libc/gen/nice.c \ + upstream-netbsd/libc/gen/popen.c \ upstream-netbsd/libc/gen/psignal.c \ upstream-netbsd/libc/gen/setjmperr.c \ upstream-netbsd/libc/gen/utime.c \ diff --git a/libc/upstream-netbsd/env.h b/libc/upstream-netbsd/env.h new file mode 100644 index 000000000..8f99f9d54 --- /dev/null +++ b/libc/upstream-netbsd/env.h @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _BIONIC_NETBSD_ENV_H_included +#define _BIONIC_NETBSD_ENV_H_included + +/* Placeholder. */ + +#endif diff --git a/libc/unistd/popen.c b/libc/upstream-netbsd/libc/gen/popen.c similarity index 57% rename from libc/unistd/popen.c rename to libc/upstream-netbsd/libc/gen/popen.c index c2355c11f..593e34631 100644 --- a/libc/unistd/popen.c +++ b/libc/upstream-netbsd/libc/gen/popen.c @@ -1,4 +1,5 @@ -/* $OpenBSD: popen.c,v 1.17 2005/08/08 08:05:34 espie Exp $ */ +/* $NetBSD: popen.c,v 1.32 2012/06/25 22:32:43 abs Exp $ */ + /* * Copyright (c) 1988, 1993 * The Regents of the University of California. All rights reserved. @@ -31,70 +32,117 @@ * SUCH DAMAGE. */ +#include +#if defined(LIBC_SCCS) && !defined(lint) +#if 0 +static char sccsid[] = "@(#)popen.c 8.3 (Berkeley) 5/3/95"; +#else +__RCSID("$NetBSD: popen.c,v 1.32 2012/06/25 22:32:43 abs Exp $"); +#endif +#endif /* LIBC_SCCS and not lint */ + +#include "namespace.h" #include #include +#include -#include +#include #include -#include +#include +#include #include #include #include -#include +#include +#include + +#include "env.h" +#include "reentrant.h" + +#ifdef __weak_alias +__weak_alias(popen,_popen) +__weak_alias(pclose,_pclose) +#endif static struct pid { struct pid *next; FILE *fp; +#ifdef _REENTRANT + int fd; +#endif pid_t pid; -} *pidlist; - -extern char **environ; +} *pidlist; + +#ifdef _REENTRANT +static rwlock_t pidlist_lock = RWLOCK_INITIALIZER; +#endif FILE * -popen(const char *program, const char *type) +popen(const char *command, const char *type) { - struct pid * volatile cur; + struct pid *cur, *old; FILE *iop; - int pdes[2]; - pid_t pid; - char *argp[] = {"sh", "-c", NULL, NULL}; + const char * volatile xtype = type; + int pdes[2], pid, serrno; + volatile int twoway; + int flags; - if ((*type != 'r' && *type != 'w') || type[1] != '\0') { - errno = EINVAL; - return (NULL); + _DIAGASSERT(command != NULL); + _DIAGASSERT(xtype != NULL); + + flags = strchr(xtype, 'e') ? O_CLOEXEC : 0; + if (strchr(xtype, '+')) { + int stype = flags ? (SOCK_STREAM | SOCK_CLOEXEC) : SOCK_STREAM; + twoway = 1; + xtype = "r+"; + if (socketpair(AF_LOCAL, stype, 0, pdes) < 0) + return NULL; + } else { + twoway = 0; + xtype = strrchr(xtype, 'r') ? "r" : "w"; + if (pipe2(pdes, flags) == -1) + return NULL; } - if ((cur = malloc(sizeof(struct pid))) == NULL) - return (NULL); - - if (pipe(pdes) < 0) { - free(cur); - return (NULL); - } - - switch (pid = fork()) { - case -1: /* Error. */ + if ((cur = malloc(sizeof(struct pid))) == NULL) { (void)close(pdes[0]); (void)close(pdes[1]); + errno = ENOMEM; + return (NULL); + } + + (void)rwlock_rdlock(&pidlist_lock); + (void)__readlockenv(); + switch (pid = vfork()) { + case -1: /* Error. */ + serrno = errno; + (void)__unlockenv(); + (void)rwlock_unlock(&pidlist_lock); free(cur); + (void)close(pdes[0]); + (void)close(pdes[1]); + errno = serrno; return (NULL); /* NOTREACHED */ case 0: /* Child. */ - { - struct pid *pcur; - /* - * We fork()'d, we got our own copy of the list, no - * contention. - */ - for (pcur = pidlist; pcur; pcur = pcur->next) - close(fileno(pcur->fp)); + /* POSIX.2 B.3.2.2 "popen() shall ensure that any streams + from previous popen() calls that remain open in the + parent process are closed in the new child process. */ + for (old = pidlist; old; old = old->next) +#ifdef _REENTRANT + close(old->fd); /* don't allow a flush */ +#else + close(fileno(old->fp)); /* don't allow a flush */ +#endif - if (*type == 'r') { - (void) close(pdes[0]); + if (*xtype == 'r') { + (void)close(pdes[0]); if (pdes[1] != STDOUT_FILENO) { (void)dup2(pdes[1], STDOUT_FILENO); (void)close(pdes[1]); } + if (twoway) + (void)dup2(STDOUT_FILENO, STDIN_FILENO); } else { (void)close(pdes[1]); if (pdes[0] != STDIN_FILENO) { @@ -102,19 +150,25 @@ popen(const char *program, const char *type) (void)close(pdes[0]); } } - argp[2] = (char *)program; - execve(_PATH_BSHELL, argp, environ); + + execl(_PATH_BSHELL, "sh", "-c", command, NULL); _exit(127); /* NOTREACHED */ - } } + (void)__unlockenv(); /* Parent; assume fdopen can't fail. */ - if (*type == 'r') { - iop = fdopen(pdes[0], type); + if (*xtype == 'r') { + iop = fdopen(pdes[0], xtype); +#ifdef _REENTRANT + cur->fd = pdes[0]; +#endif (void)close(pdes[1]); } else { - iop = fdopen(pdes[1], type); + iop = fdopen(pdes[1], xtype); +#ifdef _REENTRANT + cur->fd = pdes[1]; +#endif (void)close(pdes[0]); } @@ -123,6 +177,7 @@ popen(const char *program, const char *type) cur->pid = pid; cur->next = pidlist; pidlist = cur; + (void)rwlock_unlock(&pidlist_lock); return (iop); } @@ -139,25 +194,33 @@ pclose(FILE *iop) int pstat; pid_t pid; + _DIAGASSERT(iop != NULL); + + rwlock_wrlock(&pidlist_lock); + /* Find the appropriate file pointer. */ for (last = NULL, cur = pidlist; cur; last = cur, cur = cur->next) if (cur->fp == iop) break; - - if (cur == NULL) + if (cur == NULL) { + (void)rwlock_unlock(&pidlist_lock); return (-1); + } (void)fclose(iop); - do { - pid = waitpid(cur->pid, &pstat, 0); - } while (pid == -1 && errno == EINTR); - /* Remove the entry from the linked list. */ if (last == NULL) pidlist = cur->next; else last->next = cur->next; + + (void)rwlock_unlock(&pidlist_lock); + + do { + pid = waitpid(cur->pid, &pstat, 0); + } while (pid == -1 && errno == EINTR); + free(cur); return (pid == -1 ? -1 : pstat); diff --git a/libc/upstream-netbsd/netbsd-compat.h b/libc/upstream-netbsd/netbsd-compat.h index 3833c1d70..e33885e76 100644 --- a/libc/upstream-netbsd/netbsd-compat.h +++ b/libc/upstream-netbsd/netbsd-compat.h @@ -24,4 +24,13 @@ // TODO: update our to support this properly. #define __type_fit(t, a) (0 == 0) +// TODO: our 2.6 emulator kernels don't support SOCK_CLOEXEC yet, so we have to do without. +#define SOCK_CLOEXEC 0 + +#define _GNU_SOURCE + +// TODO: we don't yet have thread-safe environment variables. +#define __readlockenv() 0 +#define __unlockenv() 0 + #endif diff --git a/libc/upstream-netbsd/reentrant.h b/libc/upstream-netbsd/reentrant.h index a4381d479..e2945da42 100644 --- a/libc/upstream-netbsd/reentrant.h +++ b/libc/upstream-netbsd/reentrant.h @@ -17,9 +17,22 @@ #ifndef _BIONIC_NETBSD_REENTRANT_H_included #define _BIONIC_NETBSD_REENTRANT_H_included +#define _REENTRANT + #include #include -// Placeholder. +// +// Map NetBSD libc internal locking to pthread locking. +// + +#define MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER +#define mutex_t pthread_mutex_t + +#define RWLOCK_INITIALIZER PTHREAD_RWLOCK_INITIALIZER +#define rwlock_t pthread_rwlock_t +#define rwlock_rdlock pthread_rwlock_rdlock +#define rwlock_unlock pthread_rwlock_unlock +#define rwlock_wrlock pthread_rwlock_wrlock #endif diff --git a/tests/stdio_test.cpp b/tests/stdio_test.cpp index 196c725aa..7569d04fe 100644 --- a/tests/stdio_test.cpp +++ b/tests/stdio_test.cpp @@ -185,3 +185,15 @@ TEST(stdio, printf_ssize_t) { snprintf(buf, sizeof(buf), "%zd", v); #endif } + +TEST(stdio, popen) { + FILE* fp = popen("cat /proc/version", "r"); + ASSERT_TRUE(fp != NULL); + + char buf[16]; + char* s = fgets(buf, sizeof(buf), fp); + buf[13] = '\0'; + ASSERT_STREQ("Linux version", s); + + ASSERT_EQ(0, pclose(fp)); +}