Simplify tzdata fallback logic.
It's a historical accident that we try all the other files even if a
higher-priority file doesn't contain the sought-for olson id. Stop
doing that.
Also remove a TODO that has never been warranted, and add one that it
will be many years before anyone can do anything about it, but by that
time the historical knowledge may have been lost.
Bug: http://b/159613340
Test: treehugger
Change-Id: I873579268753c84b0bb721ea56f71ba64506d45a
diff --git a/libc/tzcode/bionic.cpp b/libc/tzcode/bionic.cpp
index 6d84303..182fa89 100644
--- a/libc/tzcode/bionic.cpp
+++ b/libc/tzcode/bionic.cpp
@@ -105,12 +105,18 @@
int32_t unused; // Was raw GMT offset; always 0 since tzdata2014f (L).
};
+// Returns -2 for a soft failure (where the caller should try another file),
+// -1 for a hard failure (where the caller should give up), and >= 0 is a
+// file descriptor whose offset points to the data for the given olson id in
+// the given file (and *entry_length is the size of the data).
static int __bionic_open_tzdata_path(const char* path,
const char* olson_id,
int32_t* entry_length) {
int fd = TEMP_FAILURE_RETRY(open(path, O_RDONLY | O_CLOEXEC));
if (fd == -1) {
- return -2; // Distinguish failure to find any data from failure to find a specific id.
+ // We don't log here, because this is quite common --- current devices
+ // aren't expected to have the old APK tzdata, for example.
+ return -2;
}
bionic_tzdata_header_t header = {};
@@ -119,49 +125,49 @@
fprintf(stderr, "%s: could not read header of \"%s\": %s\n",
__FUNCTION__, path, (bytes_read == -1) ? strerror(errno) : "short read");
close(fd);
- return -1;
+ return -2;
}
if (strncmp(header.tzdata_version, "tzdata", 6) != 0 || header.tzdata_version[11] != 0) {
fprintf(stderr, "%s: bad magic in \"%s\": \"%.6s\"\n", __FUNCTION__, path, header.tzdata_version);
close(fd);
- return -1;
+ return -2;
}
if (TEMP_FAILURE_RETRY(lseek(fd, ntohl(header.index_offset), SEEK_SET)) == -1) {
fprintf(stderr, "%s: couldn't seek to index in \"%s\": %s\n", __FUNCTION__, path, strerror(errno));
close(fd);
- return -1;
+ return -2;
}
if (ntohl(header.index_offset) > ntohl(header.data_offset)) {
fprintf(stderr, "%s: invalid data and index offsets in \"%s\": %u %u\n",
__FUNCTION__, path, ntohl(header.data_offset), ntohl(header.index_offset));
close(fd);
- return -1;
+ return -2;
}
const size_t index_size = ntohl(header.data_offset) - ntohl(header.index_offset);
if ((index_size % sizeof(index_entry_t)) != 0) {
fprintf(stderr, "%s: invalid index size in \"%s\": %zd\n", __FUNCTION__, path, index_size);
close(fd);
- return -1;
+ return -2;
}
- off_t specific_zone_offset = -1;
char* index = reinterpret_cast<char*>(malloc(index_size));
if (index == nullptr) {
fprintf(stderr, "%s: couldn't allocate %zd-byte index for \"%s\"\n", __FUNCTION__, index_size, path);
close(fd);
- return -1;
+ return -2;
}
if (TEMP_FAILURE_RETRY(read(fd, index, index_size)) != static_cast<ssize_t>(index_size)) {
fprintf(stderr, "%s: could not read index of \"%s\": %s\n",
__FUNCTION__, path, (bytes_read == -1) ? strerror(errno) : "short read");
free(index);
close(fd);
- return -1;
+ return -2;
}
+ off_t specific_zone_offset = -1;
size_t id_count = index_size / sizeof(index_entry_t);
index_entry_t* entry = reinterpret_cast<index_entry_t*>(index);
for (size_t i = 0; i < id_count; ++i) {
@@ -180,6 +186,9 @@
free(index);
if (specific_zone_offset == -1) {
+ // We found a valid tzdata file, but didn't find the requested id in it.
+ // Give up now, and don't try fallback tzdata files. We don't log here
+ // because for all we know the given olson id was nonsense.
close(fd);
return -1;
}
@@ -188,11 +197,9 @@
fprintf(stderr, "%s: could not seek to %ld in \"%s\": %s\n",
__FUNCTION__, specific_zone_offset, path, strerror(errno));
close(fd);
- return -1;
+ return -2;
}
- // TODO: check that there's TZ_MAGIC at this offset, so we can fall back to the other file if not.
-
return fd;
}
@@ -204,8 +211,9 @@
// tried first because it allows us to test that the time zone updates
// via APK mechanism still works even on devices with the time zone
// module.
+ // TODO: remove this when those devices are no longer supported.
// 2: The time zone data module which contains the main copy. This is the
- // common case.
+ // common case for current devices.
// 3: The ultimate fallback: the non-updatable copy in /system.
#if defined(__ANDROID__)
@@ -214,15 +222,15 @@
fd = __bionic_open_tzdata_path("/data/misc/zoneinfo/current/tzdata",
olson_id, entry_length);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
fd = __bionic_open_tzdata_path("/apex/com.android.tzdata/etc/tz/tzdata",
olson_id, entry_length);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
fd = __bionic_open_tzdata_path("/system/usr/share/zoneinfo/tzdata",
olson_id, entry_length);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
#else
// On the host, we don't expect the hard-coded locations above to exist, and
// we're not worried about security so we trust $ANDROID_DATA,
@@ -232,17 +240,17 @@
char* path = make_path("ANDROID_DATA", "/misc/zoneinfo/current/tzdata");
fd = __bionic_open_tzdata_path(path, olson_id, entry_length);
free(path);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
path = make_path("ANDROID_TZDATA_ROOT", "/etc/tz/tzdata");
fd = __bionic_open_tzdata_path(path, olson_id, entry_length);
free(path);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
path = make_path("ANDROID_ROOT", "/usr/share/zoneinfo/tzdata");
fd = __bionic_open_tzdata_path(path, olson_id, entry_length);
free(path);
- if (fd >= 0) return fd;
+ if (fd >= -1) return fd;
#endif
// Not finding any tzdata is more serious that not finding a specific zone,
@@ -253,5 +261,6 @@
fprintf(stderr, "%s: couldn't find any tzdata when looking for %s!\n", __FUNCTION__, olson_id);
}
+ // Otherwise we were successful.
return fd;
}