Stop casting uintptr_t* to int*.

BUG=130980
TEST=FilePathWatcherTest passes on 64


Review URL: https://chromiumcodereview.appspot.com/13406005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191913 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 29 insertions, 24 deletions

diff --git a/base/files/file_path_watcher.h b/base/files/file_path_watcher.h
index 896601c..3e5c7fc 100644
--- a/base/files/file_path_watcher.h
+++ b/base/files/file_path_watcher.h
@@ -17,7 +17,7 @@
 namespace base {
 
 // This class lets you register interest in changes on a FilePath.
-// The delegate will get called whenever the file or directory referenced by the
+// The callback will get called whenever the file or directory referenced by the
 // FilePath is changed, including created or deleted. Due to limitations in the
 // underlying OS APIs, FilePathWatcher has slightly different semantics on OS X
 // than on Windows or Linux. FilePathWatcher on Linux and Windows will detect
@@ -33,8 +33,6 @@ class BASE_EXPORT FilePathWatcher {
   typedef base::Callback<void(const FilePath& path, bool error)> Callback;
 
   // Used internally to encapsulate different members on different platforms.
-  // TODO(jhawkins): Move this into its own file. Also fix the confusing naming
-  // wrt Delegate vs PlatformDelegate.
   class PlatformDelegate : public base::RefCountedThreadSafe<PlatformDelegate> {
    public:
     PlatformDelegate();
diff --git a/base/files/file_path_watcher_kqueue.cc b/base/files/file_path_watcher_kqueue.cc
index c348ebd..1c086de 100644
--- a/base/files/file_path_watcher_kqueue.cc
+++ b/base/files/file_path_watcher_kqueue.cc
@@ -94,7 +94,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
                               bool* target_file_affected,
                               bool* update_watches);
 
-  // Respond to a move of deletion of the path component represented by
+  // Respond to a move or deletion of the path component represented by
   // |event|. Sets |target_file_affected| to true if |target_| is affected.
   // Sets |update_watches| to true if |events_| need to be updated.
   void HandleDeleteOrMoveChange(const EventVector::iterator& event,
@@ -123,14 +123,16 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
 
   // Returns a file descriptor that will not block the system from deleting
   // the file it references.
-  static int FileDescriptorForPath(const FilePath& path);
+  static uintptr_t FileDescriptorForPath(const FilePath& path);
+
+  static const uintptr_t kNoFileDescriptor = static_cast<uintptr_t>(-1);
 
   // Closes |*fd| and sets |*fd| to -1.
-  static void CloseFileDescriptor(int* fd);
+  static void CloseFileDescriptor(uintptr_t* fd);
 
   // Returns true if kevent has open file descriptor.
   static bool IsKeventFileDescriptorOpen(const struct kevent& event) {
-    return event.ident != static_cast<uintptr_t>(-1);
+    return event.ident != kNoFileDescriptor;
   }
 
   static EventData* EventDataForKevent(const struct kevent& event) {
@@ -148,7 +150,7 @@ class FilePathWatcherImpl : public FilePathWatcher::PlatformDelegate,
 };
 
 void FilePathWatcherImpl::ReleaseEvent(struct kevent& event) {
-  CloseFileDescriptor(reinterpret_cast<int*>(&event.ident));
+  CloseFileDescriptor(&event.ident);
   EventData* entry = EventDataForKevent(event);
   delete entry;
   event.udata = NULL;
@@ -176,10 +178,10 @@ int FilePathWatcherImpl::EventsForPath(FilePath path, EventVector* events) {
     } else {
       built_path = built_path.Append(*i);
     }
-    int fd = -1;
+    uintptr_t fd = kNoFileDescriptor;
     if (path_still_exists) {
       fd = FileDescriptorForPath(built_path);
-      if (fd == -1) {
+      if (fd == kNoFileDescriptor) {
         path_still_exists = false;
       } else {
         ++last_existing_entry;
@@ -196,19 +198,22 @@ int FilePathWatcherImpl::EventsForPath(FilePath path, EventVector* events) {
   return last_existing_entry;
 }
 
-int FilePathWatcherImpl::FileDescriptorForPath(const FilePath& path) {
-  return HANDLE_EINTR(open(path.value().c_str(), O_EVTONLY));
+uintptr_t FilePathWatcherImpl::FileDescriptorForPath(const FilePath& path) {
+  int fd = HANDLE_EINTR(open(path.value().c_str(), O_EVTONLY));
+  if (fd == -1)
+    return kNoFileDescriptor;
+  return fd;
 }
 
-void FilePathWatcherImpl::CloseFileDescriptor(int *fd) {
-  if (*fd == -1) {
+void FilePathWatcherImpl::CloseFileDescriptor(uintptr_t* fd) {
+  if (*fd == kNoFileDescriptor) {
     return;
   }
 
   if (HANDLE_EINTR(close(*fd)) != 0) {
     DPLOG(ERROR) << "close";
   }
-  *fd = -1;
+  *fd = kNoFileDescriptor;
 }
 
 bool FilePathWatcherImpl::AreKeventValuesValid(struct kevent* kevents,
@@ -236,7 +241,7 @@ bool FilePathWatcherImpl::AreKeventValuesValid(struct kevent* kevents,
       }
       if (path_name.empty()) {
         path_name = base::StringPrintf(
-            "fd %d", *reinterpret_cast<int*>(&kevents[i].ident));
+            "fd %ld", reinterpret_cast<long>(&kevents[i].ident));
       }
       DLOG(ERROR) << "Error: " << kevents[i].data << " for " << path_name;
       valid = false;
@@ -252,8 +257,8 @@ void FilePathWatcherImpl::HandleAttributesChange(
   EventVector::iterator next_event = event + 1;
   EventData* next_event_data = EventDataForKevent(*next_event);
   // Check to see if the next item in path is still accessible.
-  int have_access = FileDescriptorForPath(next_event_data->path_);
-  if (have_access == -1) {
+  uintptr_t have_access = FileDescriptorForPath(next_event_data->path_);
+  if (have_access == kNoFileDescriptor) {
     *target_file_affected = true;
     *update_watches = true;
     EventVector::iterator local_event(event);
@@ -262,7 +267,7 @@ void FilePathWatcherImpl::HandleAttributesChange(
       // potentially rendering other events in |updates| invalid.
       // There is no need to remove the events from |kqueue_| because this
       // happens as a side effect of closing the file descriptor.
-      CloseFileDescriptor(reinterpret_cast<int*>(&local_event->ident));
+      CloseFileDescriptor(&local_event->ident);
     }
   } else {
     CloseFileDescriptor(&have_access);
@@ -281,7 +286,7 @@ void FilePathWatcherImpl::HandleDeleteOrMoveChange(
     // potentially rendering other events in |updates| invalid.
     // There is no need to remove the events from |kqueue_| because this
     // happens as a side effect of closing the file descriptor.
-    CloseFileDescriptor(reinterpret_cast<int*>(&local_event->ident));
+    CloseFileDescriptor(&local_event->ident);
   }
 }
 
@@ -291,10 +296,9 @@ void FilePathWatcherImpl::HandleCreateItemChange(
     bool* update_watches) {
   // Get the next item in the path.
   EventVector::iterator next_event = event + 1;
-  EventData* next_event_data = EventDataForKevent(*next_event);
-
   // Check to see if it already has a valid file descriptor.
   if (!IsKeventFileDescriptorOpen(*next_event)) {
+    EventData* next_event_data = EventDataForKevent(*next_event);
     // If not, attempt to open a file descriptor for it.
     next_event->ident = FileDescriptorForPath(next_event_data->path_);
     if (IsKeventFileDescriptorOpen(*next_event)) {
@@ -381,7 +385,7 @@ void FilePathWatcherImpl::OnFileCanReadWithoutBlocking(int fd) {
         break;
       }
     }
-    if (!IsKeventFileDescriptorOpen(*event) || event == events_.end()) {
+    if (event == events_.end() || !IsKeventFileDescriptorOpen(*event)) {
       // The event may no longer exist in |events_| because another event
       // modified |events_| in such a way to make it invalid. For example if
       // the path is /foo/bar/bam and foo is deleted, NOTE_DELETE events for
@@ -493,7 +497,10 @@ void FilePathWatcherImpl::CancelOnMessageLoopThread() {
   if (!is_cancelled()) {
     set_cancelled();
     kqueue_watcher_.StopWatchingFileDescriptor();
-    CloseFileDescriptor(&kqueue_);
+    if (HANDLE_EINTR(close(kqueue_)) != 0) {
+      DPLOG(ERROR) << "close kqueue";
+    }
+    kqueue_ = -1;
     std::for_each(events_.begin(), events_.end(), ReleaseEvent);
     events_.clear();
     io_message_loop_ = NULL;