Merge "Fix more system/core/include warnings"
diff --git a/adb/socket_test.cpp b/adb/socket_test.cpp
index 2bb01a3..5e79b5e 100644
--- a/adb/socket_test.cpp
+++ b/adb/socket_test.cpp
@@ -307,6 +307,17 @@
// Don't register a port unless it's all numbers and ends with ':'.
VerifySkipHostSerial(protocol + "foo:123", ":123");
VerifySkipHostSerial(protocol + "foo:123bar:baz", ":123bar:baz");
+
+ VerifySkipHostSerial(protocol + "100.100.100.100:5555:foo", ":foo");
+ VerifySkipHostSerial(protocol + "[0123:4567:89ab:CDEF:0:9:a:f]:5555:foo", ":foo");
+ VerifySkipHostSerial(protocol + "[::1]:5555:foo", ":foo");
+
+ // If we can't find both [] then treat it as a normal serial with [ in it.
+ VerifySkipHostSerial(protocol + "[0123:foo", ":foo");
+
+ // Don't be fooled by random IPv6 addresses in the command string.
+ VerifySkipHostSerial(protocol + "foo:ping [0123:4567:89ab:CDEF:0:9:a:f]:5555",
+ ":ping [0123:4567:89ab:CDEF:0:9:a:f]:5555");
}
}
diff --git a/adb/sockets.cpp b/adb/sockets.cpp
index 503e018..b809c4f 100644
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -622,21 +622,32 @@
service += 4;
}
- char* first_colon = strchr(service, ':');
- if (!first_colon) {
+ // Check for an IPv6 address. `adb connect` creates the serial number from the canonical
+ // network address so it will always have the [] delimiters.
+ if (service[0] == '[') {
+ char* ipv6_end = strchr(service, ']');
+ if (ipv6_end != nullptr) {
+ service = ipv6_end;
+ }
+ }
+
+ // The next colon we find must either begin the port field or the command field.
+ char* colon_ptr = strchr(service, ':');
+ if (!colon_ptr) {
// No colon in service string.
return nullptr;
}
- char* serial_end = first_colon;
+ // If the next field is only decimal digits and ends with another colon, it's a port.
+ char* serial_end = colon_ptr;
if (isdigit(serial_end[1])) {
serial_end++;
while (*serial_end && isdigit(*serial_end)) {
serial_end++;
}
if (*serial_end != ':') {
- // Something other than numbers was found, reset the end.
- serial_end = first_colon;
+ // Something other than "<port>:" was found, this must be the command field instead.
+ serial_end = colon_ptr;
}
}
return serial_end;
diff --git a/include/log/event_tag_map.h b/include/log/event_tag_map.h
index 1653c61..7ee0a7c 100644
--- a/include/log/event_tag_map.h
+++ b/include/log/event_tag_map.h
@@ -41,7 +41,7 @@
/*
* Look up a tag by index. Returns the tag string, or NULL if not found.
*/
-const char* android_lookupEventTag(const EventTagMap* map, int tag);
+const char* android_lookupEventTag(const EventTagMap* map, unsigned int tag);
#ifdef __cplusplus
}
diff --git a/liblog/event_tag_map.c b/liblog/event_tag_map.c
index 345f0d3..c8943e0 100644
--- a/liblog/event_tag_map.c
+++ b/liblog/event_tag_map.c
@@ -15,8 +15,10 @@
*/
#include <assert.h>
+#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
+#include <inttypes.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
@@ -32,8 +34,8 @@
* Single entry.
*/
typedef struct EventTag {
- unsigned int tagIndex;
- const char* tagStr;
+ uint32_t tagIndex;
+ const char* tagStr;
} EventTag;
/*
@@ -56,7 +58,6 @@
static int scanTagLine(char** pData, EventTag* tag, int lineNum);
static int sortTags(EventTagMap* map);
-
/*
* Open the map file and allocate a structure to manage it.
*
@@ -67,47 +68,57 @@
{
EventTagMap* newTagMap;
off_t end;
- int fd = -1;
+ int save_errno;
- newTagMap = calloc(1, sizeof(EventTagMap));
- if (newTagMap == NULL)
- return NULL;
-
- fd = open(fileName, O_RDONLY | O_CLOEXEC);
+ int fd = open(fileName, O_RDONLY | O_CLOEXEC);
if (fd < 0) {
+ save_errno = errno;
fprintf(stderr, "%s: unable to open map '%s': %s\n",
- OUT_TAG, fileName, strerror(errno));
- goto fail;
+ OUT_TAG, fileName, strerror(save_errno));
+ goto fail_errno;
}
end = lseek(fd, 0L, SEEK_END);
+ save_errno = errno;
(void) lseek(fd, 0L, SEEK_SET);
if (end < 0) {
- fprintf(stderr, "%s: unable to seek map '%s'\n", OUT_TAG, fileName);
- goto fail;
+ fprintf(stderr, "%s: unable to seek map '%s' %s\n",
+ OUT_TAG, fileName, strerror(save_errno));
+ goto fail_close;
}
- newTagMap->mapAddr = mmap(NULL, end, PROT_READ | PROT_WRITE, MAP_PRIVATE,
- fd, 0);
- if (newTagMap->mapAddr == MAP_FAILED) {
- fprintf(stderr, "%s: mmap(%s) failed: %s\n",
- OUT_TAG, fileName, strerror(errno));
- goto fail;
+ newTagMap = (EventTagMap*)calloc(1, sizeof(EventTagMap));
+ if (newTagMap == NULL) {
+ save_errno = errno;
+ goto fail_close;
}
+
+ newTagMap->mapAddr = mmap(NULL, end, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+ save_errno = errno;
+ close(fd);
+ fd = -1;
+ if ((newTagMap->mapAddr == MAP_FAILED) || (newTagMap->mapAddr == NULL)) {
+ fprintf(stderr, "%s: mmap(%s) failed: %s\n",
+ OUT_TAG, fileName, strerror(save_errno));
+ goto fail_free;
+ }
+
newTagMap->mapLen = end;
- if (processFile(newTagMap) != 0)
- goto fail;
-
- if (fd >= 0)
- close(fd);
+ if (processFile(newTagMap) != 0) goto fail_unmap;
return newTagMap;
+fail_unmap:
+ munmap(newTagMap->mapAddr, newTagMap->mapLen);
+ save_errno = EINVAL;
+fail_free:
+ free(newTagMap);
+fail_close:
+ close(fd);
+fail_errno:
+ errno = save_errno;
fail:
- android_closeEventTagMap(newTagMap);
- if (fd >= 0)
- close(fd);
return NULL;
}
@@ -116,10 +127,10 @@
*/
LIBLOG_ABI_PUBLIC void android_closeEventTagMap(EventTagMap* map)
{
- if (map == NULL)
- return;
+ if (map == NULL) return;
munmap(map->mapAddr, map->mapLen);
+ free(map->tagArray);
free(map);
}
@@ -129,18 +140,15 @@
* The entries are sorted by tag number, so we can do a binary search.
*/
LIBLOG_ABI_PUBLIC const char* android_lookupEventTag(const EventTagMap* map,
- int tag)
+ unsigned int tag)
{
- int hi, lo, mid;
-
- lo = 0;
- hi = map->numTags-1;
+ int lo = 0;
+ int hi = map->numTags - 1;
while (lo <= hi) {
- int cmp;
+ int mid = (lo + hi) / 2;
+ int cmp = map->tagArray[mid].tagIndex - tag;
- mid = (lo+hi)/2;
- cmp = map->tagArray[mid].tagIndex - tag;
if (cmp < 0) {
/* tag is bigger */
lo = mid + 1;
@@ -153,39 +161,10 @@
}
}
+ errno = ENOENT;
return NULL;
}
-
-
-/*
- * Determine whether "c" is a whitespace char.
- */
-static inline int isCharWhitespace(char c)
-{
- return (c == ' ' || c == '\n' || c == '\r' || c == '\t');
-}
-
-/*
- * Determine whether "c" is a valid tag char.
- */
-static inline int isCharValidTag(char c)
-{
- return ((c >= 'A' && c <= 'Z') ||
- (c >= 'a' && c <= 'z') ||
- (c >= '0' && c <= '9') ||
- (c == '_'));
-}
-
-/*
- * Determine whether "c" is a valid decimal digit.
- */
-static inline int isCharDigit(char c)
-{
- return (c >= '0' && c <= '9');
-}
-
-
/*
* Crunch through the file, parsing the contents and creating a tag index.
*/
@@ -193,25 +172,20 @@
{
/* get a tag count */
map->numTags = countMapLines(map);
- if (map->numTags < 0)
- return -1;
-
- //printf("+++ found %d tags\n", map->numTags);
-
- /* allocate storage for the tag index array */
- map->tagArray = calloc(1, sizeof(EventTag) * map->numTags);
- if (map->tagArray == NULL)
- return -1;
-
- /* parse the file, null-terminating tag strings */
- if (parseMapLines(map) != 0) {
- fprintf(stderr, "%s: file parse failed\n", OUT_TAG);
+ if (map->numTags < 0) {
+ errno = ENOENT;
return -1;
}
+ /* allocate storage for the tag index array */
+ map->tagArray = (EventTag*)calloc(1, sizeof(EventTag) * map->numTags);
+ if (map->tagArray == NULL) return -1;
+
+ /* parse the file, null-terminating tag strings */
+ if (parseMapLines(map) != 0) return -1;
+
/* sort the tags and check for duplicates */
- if (sortTags(map) != 0)
- return -1;
+ if (sortTags(map) != 0) return -1;
return 0;
}
@@ -228,24 +202,20 @@
*/
static int countMapLines(const EventTagMap* map)
{
- int numTags, unknown;
- const char* cp;
- const char* endp;
+ const char* cp = (const char*) map->mapAddr;
+ const char* endp = cp + map->mapLen;
+ int numTags = 0;
+ int unknown = 1;
- cp = (const char*) map->mapAddr;
- endp = cp + map->mapLen;
-
- numTags = 0;
- unknown = 1;
while (cp < endp) {
if (*cp == '\n') {
unknown = 1;
} else if (unknown) {
- if (isCharDigit(*cp)) {
+ if (isdigit(*cp)) {
/* looks like a tag to me */
numTags++;
unknown = 0;
- } else if (isCharWhitespace(*cp)) {
+ } else if (isspace(*cp)) {
/* might be leading whitespace before tag num, keep going */
} else {
/* assume comment; second pass can complain in detail */
@@ -266,15 +236,13 @@
static int parseMapLines(EventTagMap* map)
{
int tagNum, lineStart, lineNum;
- char* cp;
- char* endp;
-
- cp = (char*) map->mapAddr;
- endp = cp + map->mapLen;
+ char* cp = (char*) map->mapAddr;
+ char* endp = cp + map->mapLen;
/* insist on EOL at EOF; simplifies parsing and null-termination */
- if (*(endp-1) != '\n') {
+ if (*(endp - 1) != '\n') {
fprintf(stderr, "%s: map file missing EOL on last line\n", OUT_TAG);
+ errno = EINVAL;
return -1;
}
@@ -282,7 +250,6 @@
lineStart = 1;
lineNum = 1;
while (cp < endp) {
- //printf("{%02x}", *cp); fflush(stdout);
if (*cp == '\n') {
lineStart = 1;
lineNum++;
@@ -290,24 +257,27 @@
if (*cp == '#') {
/* comment; just scan to end */
lineStart = 0;
- } else if (isCharDigit(*cp)) {
+ } else if (isdigit(*cp)) {
/* looks like a tag; scan it out */
if (tagNum >= map->numTags) {
fprintf(stderr,
"%s: more tags than expected (%d)\n", OUT_TAG, tagNum);
+ errno = EMFILE;
return -1;
}
- if (scanTagLine(&cp, &map->tagArray[tagNum], lineNum) != 0)
+ if (scanTagLine(&cp, &map->tagArray[tagNum], lineNum) != 0) {
return -1;
+ }
tagNum++;
lineNum++; // we eat the '\n'
/* leave lineStart==1 */
- } else if (isCharWhitespace(*cp)) {
+ } else if (isspace(*cp)) {
/* looks like leading whitespace; keep scanning */
} else {
fprintf(stderr,
"%s: unexpected chars (0x%02x) in tag number on line %d\n",
OUT_TAG, *cp, lineNum);
+ errno = EINVAL;
return -1;
}
} else {
@@ -319,6 +289,7 @@
if (tagNum != map->numTags) {
fprintf(stderr, "%s: parsed %d tags, expected %d\n",
OUT_TAG, tagNum, map->numTags);
+ errno = EINVAL;
return -1;
}
@@ -336,41 +307,41 @@
*/
static int scanTagLine(char** pData, EventTag* tag, int lineNum)
{
- char* cp = *pData;
- char* startp;
- char* endp;
- unsigned long val;
+ char* cp;
- startp = cp;
- while (isCharDigit(*++cp))
- ;
- *cp = '\0';
-
- val = strtoul(startp, &endp, 10);
- assert(endp == cp);
- if (endp != cp)
- fprintf(stderr, "ARRRRGH\n");
+ unsigned long val = strtoul(*pData, &cp, 10);
+ if (cp == *pData) {
+ fprintf(stderr, "%s: malformed tag number on line %d\n", OUT_TAG, lineNum);
+ errno = EINVAL;
+ return -1;
+ }
tag->tagIndex = val;
+ if (tag->tagIndex != val) {
+ fprintf(stderr, "%s: tag number too large on line %d\n", OUT_TAG, lineNum);
+ errno = ERANGE;
+ return -1;
+ }
- while (*++cp != '\n' && isCharWhitespace(*cp))
- ;
+ while ((*++cp != '\n') && isspace(*cp)) {
+ }
if (*cp == '\n') {
- fprintf(stderr,
- "%s: missing tag string on line %d\n", OUT_TAG, lineNum);
+ fprintf(stderr, "%s: missing tag string on line %d\n", OUT_TAG, lineNum);
+ errno = EINVAL;
return -1;
}
tag->tagStr = cp;
- while (isCharValidTag(*++cp))
- ;
+ /* Determine whether "c" is a valid tag char. */
+ while (isalnum(*++cp) || (*cp == '_')) {
+ }
if (*cp == '\n') {
/* null terminate and return */
*cp = '\0';
- } else if (isCharWhitespace(*cp)) {
+ } else if (isspace(*cp)) {
/* CRLF or trailin spaces; zap this char, then scan for the '\n' */
*cp = '\0';
@@ -380,14 +351,13 @@
while (*++cp != '\n') {
}
} else {
- fprintf(stderr,
- "%s: invalid tag chars on line %d\n", OUT_TAG, lineNum);
+ fprintf(stderr, "%s: invalid tag chars on line %d\n", OUT_TAG, lineNum);
+ errno = EINVAL;
return -1;
}
*pData = cp;
- //printf("+++ Line %d: got %d '%s'\n", lineNum, tag->tagIndex, tag->tagStr);
return 0;
}
@@ -415,11 +385,13 @@
qsort(map->tagArray, map->numTags, sizeof(EventTag), compareEventTags);
for (i = 1; i < map->numTags; i++) {
- if (map->tagArray[i].tagIndex == map->tagArray[i-1].tagIndex) {
- fprintf(stderr, "%s: duplicate tag entries (%d:%s and %d:%s)\n",
+ if (map->tagArray[i].tagIndex == map->tagArray[i - 1].tagIndex) {
+ fprintf(stderr,
+ "%s: duplicate tag entries (%" PRIu32 ":%s and %" PRIu32 ":%s)\n",
OUT_TAG,
map->tagArray[i].tagIndex, map->tagArray[i].tagStr,
- map->tagArray[i-1].tagIndex, map->tagArray[i-1].tagStr);
+ map->tagArray[i - 1].tagIndex, map->tagArray[i - 1].tagStr);
+ errno = EMLINK;
return -1;
}
}
diff --git a/liblog/logger_write.c b/liblog/logger_write.c
index c7b5a84..08e6348 100644
--- a/liblog/logger_write.c
+++ b/liblog/logger_write.c
@@ -132,12 +132,20 @@
}
return kLogNotAvailable;
}
+
+#if defined(__BIONIC__)
+static atomic_uintptr_t tagMap;
+#endif
+
/*
* Release any logger resources. A new log write will immediately re-acquire.
*/
LIBLOG_ABI_PUBLIC void __android_log_close()
{
struct android_log_transport_write *transport;
+#if defined(__BIONIC__)
+ EventTagMap *m;
+#endif
__android_log_lock();
@@ -165,7 +173,28 @@
}
}
+#if defined(__BIONIC__)
+ /*
+ * Additional risk here somewhat mitigated by immediately unlock flushing
+ * the processor cache. The multi-threaded race that we choose to accept,
+ * to minimize locking, is an atomic_load in a writer picking up a value
+ * just prior to entering this routine. There will be an use after free.
+ *
+ * Again, anyone calling this is doing so to release the logging resources
+ * is most probably going to quiesce then shut down; or to restart after
+ * a fork so the risk should be non-existent. For this reason we
+ * choose a mitigation stance for efficiency instead of incuring the cost
+ * of a lock for every log write.
+ */
+ m = (EventTagMap *)atomic_exchange(&tagMap, (uintptr_t)0);
+#endif
+
__android_log_unlock();
+
+#if defined(__BIONIC__)
+ android_closeEventTagMap(m);
+#endif
+
}
/* log_init_lock assumed */
@@ -250,7 +279,6 @@
return -EPERM;
}
} else if (log_id == LOG_ID_EVENTS) {
- static atomic_uintptr_t map;
const char *tag;
EventTagMap *m, *f;
@@ -260,11 +288,11 @@
tag = NULL;
f = NULL;
- m = (EventTagMap *)atomic_load(&map);
+ m = (EventTagMap *)atomic_load(&tagMap);
if (!m) {
ret = __android_log_trylock();
- m = (EventTagMap *)atomic_load(&map); /* trylock flush cache */
+ m = (EventTagMap *)atomic_load(&tagMap); /* trylock flush cache */
if (!m) {
m = android_openEventTagMap(EVENT_TAG_MAP_FILE);
if (ret) { /* trylock failed, use local copy, mark for close */
@@ -273,7 +301,7 @@
if (!m) { /* One chance to open map file */
m = (EventTagMap *)(uintptr_t)-1LL;
}
- atomic_store(&map, (uintptr_t)m);
+ atomic_store(&tagMap, (uintptr_t)m);
}
}
if (!ret) { /* trylock succeeded, unlock */
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 65bbc1e..7c15429 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -203,9 +203,8 @@
{
std::unique_ptr<DIR, decltype(&closedir)> uid(opendir(uid_path), closedir);
if (uid != NULL) {
- struct dirent cur;
- struct dirent *dir;
- while ((readdir_r(uid.get(), &cur, &dir) == 0) && dir) {
+ dirent* dir;
+ while ((dir = readdir(uid.get())) != nullptr) {
char path[PROCESSGROUP_MAX_PATH_LEN];
if (dir->d_type != DT_DIR) {
@@ -231,9 +230,8 @@
if (root == NULL) {
PLOG(ERROR) << "failed to open " << cgroup_root_path;
} else {
- struct dirent cur;
- struct dirent *dir;
- while ((readdir_r(root.get(), &cur, &dir) == 0) && dir) {
+ dirent* dir;
+ while ((dir = readdir(root.get())) != nullptr) {
char path[PROCESSGROUP_MAX_PATH_LEN];
if (dir->d_type != DT_DIR) {
diff --git a/libutils/ProcessCallStack.cpp b/libutils/ProcessCallStack.cpp
index 4e87a98..73ed4eb 100644
--- a/libutils/ProcessCallStack.cpp
+++ b/libutils/ProcessCallStack.cpp
@@ -131,9 +131,6 @@
}
void ProcessCallStack::update() {
- struct dirent *ep;
- struct dirent entry;
-
std::unique_ptr<DIR, decltype(&closedir)> dp(opendir(PATH_SELF_TASK), closedir);
if (dp == NULL) {
ALOGE("%s: Failed to update the process's call stacks: %s",
@@ -158,8 +155,8 @@
* Each tid is a directory inside of /proc/self/task
* - Read every file in directory => get every tid
*/
- int code;
- while ((code = readdir_r(dp.get(), &entry, &ep)) == 0 && ep != NULL) {
+ dirent* ep;
+ while ((ep = readdir(dp.get())) != NULL) {
pid_t tid = -1;
sscanf(ep->d_name, "%d", &tid);
@@ -194,10 +191,6 @@
ALOGV("%s: Got call stack for tid %d (size %zu)",
__FUNCTION__, tid, threadInfo.callStack.size());
}
- if (code != 0) { // returns positive error value on error
- ALOGE("%s: Failed to readdir from %s: %s",
- __FUNCTION__, PATH_SELF_TASK, strerror(code));
- }
}
void ProcessCallStack::log(const char* logtag, android_LogPriority priority,