0

Revert of Replace some deprecated usages of readdir_r with readdir (patchset id:80001 of https://codereview.chromium.org/2411833004/ )

Reason for revert:
Speculative: appears to be causing flaky crashes in android_webview_test_apk and adversely affecting the CQ as a result.

e.g. https://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/36829

The last few messages in the logcat include:

Device(06ec840ef0e949a6) 11-01 01:44:07.189 27745 27808 E chromium: [ERROR:simple_index_file.cc(143)] Could not get file info for /data/data/org.chromium.android_webview.shell/cache/org.chromium.android_webview/137034c0a767955d_0
Device(06ec840ef0e949a6) 11-01 01:44:07.189 27745 27808 E chromium: [ERROR:simple_index_file_posix.cc(52)] readdir /data/data/org.chromium.android_webview.shell/cache/org.chromium.android_webview: No such file or directory
Device(06ec840ef0e949a6) 11-01 01:44:07.189 27745 27808 E chromium: [ERROR:simple_index_file.cc(544)] Could not reconstruct index from disk
Device(06ec840ef0e949a6) 11-01 01:44:07.209 27745 27788 F chromium: [FATAL:simple_index.cc(404)] Check failed: load_result->did_load.

I'll try to get a fully symbolized trace tomorrow.

Original issue's description:
> Replace some deprecated usages of readdir_r with readdir
>
> readdir_r is deprecated and using it with recent compilers cause some
> warning that may be treated as build errors. readdir is recommended
> instead, see http://man7.org/linux/man-pages/man3/readdir_r.3.html
>
> This CL replaces some usages of readdir_r with readdir in order to fix
> build errors with use_ozone=1.
>
> R=jln@chromium.org,gavinp@chromium.org,dcheng@chromium.org
> BUG=457759
>
> Committed: https://crrev.com/c2fdc2710b700158ad46111be664eac7ede0fdce
> Cr-Commit-Position: refs/heads/master@{#428867}

TBR=dcheng@chromium.org,gavinp@chromium.org,jln@chromium.org,jorgelo@chromium.org,bbudge@google.com,tonikitoo@chromium.org,fwang@igalia.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=457759

Review-Url: https://codereview.chromium.org/2466773003
Cr-Commit-Position: refs/heads/master@{#428934}
This commit is contained in:
jbudorick
2016-10-31 21:20:58 -07:00
committed by Commit bot
parent f366ebae93
commit 74b29880d2
3 changed files with 16 additions and 40 deletions
base/files
net/disk_cache/simple
sandbox/linux/services

@ -131,11 +131,9 @@ bool FileEnumerator::ReadDirectory(std::vector<FileInfo>* entries,
additional space for pathname may be needed
#endif
// In all implementations of the C library that Chromium can run with,
// concurrent calls to readdir that specify different directory streams are
// thread-safe. This is the case here, since the directory stream is scoped to
// the current function. See https://codereview.chromium.org/2411833004/#msg3
for (struct dirent* dent = readdir(dir); dent; dent = readdir(dir)) {
struct dirent dent_buf;
struct dirent* dent;
while (readdir_r(dir, &dent_buf, &dent) == 0 && dent) {
FileInfo info;
info.filename_ = FilePath(dent->d_name);

@ -34,22 +34,18 @@ bool SimpleIndexFile::TraverseCacheDirectory(
PLOG(ERROR) << "opendir " << cache_path.value();
return false;
}
// In all implementations of the C library that Chromium can run with,
// concurrent calls to readdir that specify different directory streams are
// thread-safe. This is the case here, since the directory stream is scoped to
// the current function. See https://codereview.chromium.org/2411833004/#msg3
errno = 0;
for (dirent* entry = readdir(dir.get()); entry; entry = readdir(dir.get())) {
const std::string file_name(entry->d_name);
dirent entry, *result;
while (readdir_r(dir.get(), &entry, &result) == 0) {
if (!result)
return true; // The traversal completed successfully.
const std::string file_name(result->d_name);
if (file_name == "." || file_name == "..")
continue;
const base::FilePath file_path = cache_path.Append(
base::FilePath(file_name));
entry_file_callback.Run(file_path);
}
if (!errno)
return true; // The traversal completed successfully.
PLOG(ERROR) << "readdir " << cache_path.value();
PLOG(ERROR) << "readdir_r " << cache_path.value();
return false;
}

@ -51,24 +51,15 @@ int ProcUtil::CountOpenFds(int proc_fd) {
CHECK(dir);
int count = 0;
struct dirent* de;
#if defined(OS_NACL_NONSFI)
// NaCl has not implemented readdir.
struct dirent e;
struct dirent* de;
while (!readdir_r(dir.get(), &e, &de) && de) {
#else
// In all implementations of the C library that Chromium can run with,
// concurrent calls to readdir that specify different directory streams are
// thread-safe. This is the case here, since the directory stream is scoped to
// the current function. See https://codereview.chromium.org/2411833004/#msg3
for (de = readdir(dir.get()); de; de = readdir(dir.get())) {
#endif
if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) {
if (strcmp(e.d_name, ".") == 0 || strcmp(e.d_name, "..") == 0) {
continue;
}
int fd_num;
CHECK(base::StringToInt(de->d_name, &fd_num));
CHECK(base::StringToInt(e.d_name, &fd_num));
if (fd_num == proc_fd || fd_num == proc_self_fd) {
continue;
}
@ -90,31 +81,22 @@ bool ProcUtil::HasOpenDirectory(int proc_fd) {
ScopedDIR dir(fdopendir(proc_self_fd));
CHECK(dir);
struct dirent* de;
#if defined(OS_NACL_NONSFI)
// NaCl has not implemented readdir.
struct dirent e;
struct dirent* de;
while (!readdir_r(dir.get(), &e, &de) && de) {
#else
// In all implementations of the C library that Chromium can run with,
// concurrent calls to readdir that specify different directory streams are
// thread-safe. This is the case here, since the directory stream is scoped to
// the current function. See https://codereview.chromium.org/2411833004/#msg3
for (de = readdir(dir.get()); de; de = readdir(dir.get())) {
#endif
if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) {
if (strcmp(e.d_name, ".") == 0 || strcmp(e.d_name, "..") == 0) {
continue;
}
int fd_num;
CHECK(base::StringToInt(de->d_name, &fd_num));
CHECK(base::StringToInt(e.d_name, &fd_num));
if (fd_num == proc_fd || fd_num == proc_self_fd) {
continue;
}
struct stat s;
// It's OK to use proc_self_fd here, fstatat won't modify it.
CHECK(fstatat(proc_self_fd, de->d_name, &s, 0) == 0);
CHECK(fstatat(proc_self_fd, e.d_name, &s, 0) == 0);
if (S_ISDIR(s.st_mode)) {
return true;
}