Upstreaming diffs in os_compat_android.cc

base/os_compat_android.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/os_compat_android.h"
 
+#include <errno.h>
+#include <math.h>
+#include <sys/stat.h>
 #include <time64.h>
 
 #include "base/stringprintf.h"
 
 // There is no futimes() avaiable in Bionic, so we provide our own
 // implementation until it is there.
 extern "C" {
 
 int futimes(int fd, const struct timeval tv[2]) {
   const std::string fd_path = StringPrintf("/proc/self/fd/%d", fd);
   return utimes(fd_path.c_str(), tv);
 }
 
 // Android has only timegm64() and no timegm().
 // We replicate the behaviour of timegm() when the result overflows time_t.
 time_t timegm(struct tm* const t) {
   // time_t is signed on Android.
   static const time_t kTimeMax = ~(1 << (sizeof(time_t) * CHAR_BIT - 1));
   static const time_t kTimeMin = (1 << (sizeof(time_t) * CHAR_BIT - 1));
   time64_t result = timegm64(t);
   if (result < kTimeMin || result > kTimeMax)
     return -1;
   return result;
 }
 
+// The following is only needed when building with GCC 4.6 or higher
+// (i.e. not with Android GCC 4.4.3, nor with Clang).
+//
+// GCC is now capable of optimizing successive calls to sin() and cos() into
+// a single call to sincos(). This means that source code that looks like:
+//
+//     double c, s;
+//     c = cos(angle);
+//     s = sin(angle);
+//
+// Will generate machine code that looks like:
+//
+//     double c, s;
+//     sincos(angle, &s, &c);
+//
+// Unfortunately, sincos() and friends are not part of the Android libm.so
+// library provided by the NDK for API level 9. When the optimization kicks
+// in, it makes the final build fail with a puzzling message (puzzling
+// because 'sincos' doesn't appear anywhere in the sources!).
+//
+// To solve this, we provide our own implementation of the sincos() function
+// and related friends. Note that we must also explicitely tell GCC to disable
+// optimizations when generating these. Otherwise, the generated machine code
+// for each function would simply end up calling itself, resulting in a
+// runtime crash due to stack overflow.
+//
+#if defined(__GNUC__) && !defined(__clang__)
+
+// For the record, Clang does not support the 'optimize' attribute.
+// In the unlikely event that it begins performing this optimization too,
+// we'll have to find a different way to achieve this. NOTE: Tested with O1
+// which still performs the optimization.
+//
+#define GCC_NO_OPTIMIZE  __attribute__((optimize("O0")))
+
+GCC_NO_OPTIMIZE
+void sincos(double angle, double* s, double *c)
+{

[brettw, 2012/07/10 20:56:04]

Style: Check placement of { throughout this patch

[felipeg, 2012/07/11 15:26:24]

Done.

+  *c = cos(angle);
+  *s = sin(angle);
+}
+
+GCC_NO_OPTIMIZE
+void sincosf(float angle, float* s, float* c)
+{
+  *c = cosf(angle);
+  *s = sinf(angle);
+}
+
+#endif // __GNUC__ && !__clang__
+
+// An implementation of mkdtemp, since it is not exposed by the NDK
+// for native API level 9 that we target.
+char* mkdtemp(char* path)
+{
+  if (path == NULL) {
+    errno = EINVAL;
+    return NULL;
+  }
+
+  int len = strlen(path);
+  int suffix = len;
+
+  // The last six characters of 'path' must be XXXXXX, but the string
+  // can be terminated by a longer string of 'X'. Find its start at
+  // 'suffix'.
+  while (suffix > 0 && path[suffix-1] == 'X')
+    suffix--;
+
+  // Error if there aren't at least 6 'X' characters at the end
+  if (len - suffix < 6) {
+    errno = EINVAL;
+    return NULL;
+  }
+
+  // If the path contains a directory, as in /tmp/foo/XXXXXXXX, make sure
+  // that /tmp/foo exists, otherwise we're going to loop a really long
+  // time for nothing below
+  char* dirsep = strrchr(path, '/');
+  if (dirsep != NULL) {
+    struct stat st;
+    int ret;
+
+    *dirsep = '\0'; // Terminating directory path temporarily

[brettw, 2012/07/10 20:56:04]

Style: two spaces before end-of-line comments. Check also below.

[felipeg, 2012/07/11 15:26:24]

Done.

+
+    ret = stat(path, &st);
+
+    *dirsep = '/'; // Restoring directory separator
+    if (ret < 0) {  // Directory probably does not exist
+      return NULL;
+    }

[brettw, 2012/07/10 20:56:04]

Don't use {} here since you don't elsewhere.

[felipeg, 2012/07/11 15:26:24]

Done.

+    if (!S_ISDIR(st.st_mode)) {  // Not a directory
+      errno = ENOTDIR;
+      return NULL;
+    }
+  }
+
+  // Now replace all the XXXXXs with a random alphanumerical string. We're
+  // going to use something like <randomchars><pid>, where <randomchars> is
+  // a string of random alphabetical characters, and <pid> is the decimal
+  // pid string. This matches the BSD implementation.

[brettw, 2012/07/10 20:56:04]

I wonder if all of this is necessary. The man page says the last 6 chars must be
XXXXXX. Would it be sufficient to just do:

  std::string rand_part = IntToString(RandInt(0, 1000000));

and then copy that over the 6 XXXXXX at the end and save a lot of subtle code.
We can verify beforehand that there are 6 X's and return early to make the later
logic easier.

[digit1, 2012/07/11 15:40:48]

First, for the record, this code is taken from recent version of the Android C
library (hence the C-style cast and slightly different coding style). Nothing
that can't be fixed though.

On 2012/07/10 20:56:04, brettw wrote:
Given that there can be a lot more than six Xs in the template string, and all
of them must be replaced, this won't be enough. It's true that writing the tid
value at the end is not necessary, but it's generally helpful when debugging the
origin of stale temporary files.

Besides, using std::string() and calling a base/ function from what is a
C-library-layer function seems a bit overkill.

+  pid_t pid = gettid();
+  int pos = len - 1;
+
+  // First, write the pid from last to first digit
+  while (pos >= suffix && pid != 0) {
+    path[pos--] = '0' + (pid % 10);
+    pid /= 10;
+  }
+
+  // Then, add some random chars until we cover all the XXXXs
+  while (pos >= suffix) {
+    int  ch = (arc4random() & 0xffff) % (26+26);
+    if (ch < 26)
+      ch = 'A' + ch;
+    else
+      ch = 'a' + (ch - 26);
+
+    path[pos--] = (char)ch;

[brettw, 2012/07/10 20:56:04]

Don't use C casts, use static_cast.

[felipeg, 2012/07/11 15:26:24]

Done.

+  }
+
+  // Now loop until we CAN create a directory by that name. Otherwise,
+  // 'increment' the characters after suffix to find a free one.
+  for (;;) {
+
+    if (mkdir(path, 0700) == 0) {

[brettw, 2012/07/10 20:56:04]

I'd skip the blank line above here.

[felipeg, 2012/07/11 15:26:24]

Done.

+      // We just created the directory succesfully.
+      return path;
+    }
+
+    if (errno != EEXIST) {
+      // The directory doesn't exist, but an error occured
+      return NULL;
+    }
+
+    // The directory does exist, find a new name by 'incrementing' from
+    // left to right, the rules are:
+    //   A -> B
+    //   B -> C
+    //   ...
+    //   Z -> a
+    //   a -> b
+    //   b -> c
+    //   ...
+    //   z -> A + increment the next character
+    //
+    //   0..9 -> A
+    pos = suffix;
+    for (;;) {
+      int ch = path[pos];
+      if (ch == 'Z') {   // Z -> a
+        path[pos] = 'a';
+        break;
+      }
+      if (ch == 'z') {   // z -> A + increment the next character
+        path[pos++] = 'A';
+        if (pos == len) {
+          // uh-oh, we're out of candidates here
+          return 0;
+        }
+        continue;
+      }
+      if (ch >= '0' && ch <= '9') { // 0...9 -> A
+        path[pos] = 'A';
+        break;
+      }
+      path[pos] += 1;  // otherwise:   $ch -> $ch + 1
+      break;
+    }
+  }
+}
+
 }  // extern "C"