Bug 728171 - Use Scoped.h throughout the code. r=cjones
☠☠ backed out by 12e42fb8e321 ☠ ☠
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Wed, 11 Apr 2012 18:59:10 +0200
changeset 91463 0e03eb171e0869596901d795e95077fb39d16a14
parent 91462 26eb08593f890f4b04697805dd4c29c6fad2d961
child 91464 37e9c8655cfd13883b22ac54490f6f9a0b5a636e
push id22445
push usereakhgari@mozilla.com
push dateThu, 12 Apr 2012 16:19:55 +0000
treeherdermozilla-central@901dfde60183 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscjones
bugs728171
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 728171 - Use Scoped.h throughout the code. r=cjones
dom/plugins/ipc/PluginModuleParent.cpp
gfx/src/X11Util.h
ipc/ril/Ril.cpp
modules/libjar/nsZipArchive.cpp
toolkit/components/telemetry/Telemetry.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
widget/gonk/Framebuffer.cpp
widget/gtk2/nsSound.cpp
xpcom/build/FileLocation.cpp
xpcom/glue/FileUtils.h
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -553,19 +553,21 @@ PluginModuleParent::NPP_SetValue(NPP ins
 }
 
 bool
 PluginModuleParent::RecvBackUpXResources(const FileDescriptor& aXSocketFd)
 {
 #ifndef MOZ_X11
     NS_RUNTIMEABORT("This message only makes sense on X11 platforms");
 #else
-    NS_ABORT_IF_FALSE(0 > mPluginXSocketFdDup.mFd,
+    NS_ABORT_IF_FALSE(0 > mPluginXSocketFdDup.get(),
                       "Already backed up X resources??");
-    mPluginXSocketFdDup.mFd = aXSocketFd.fd;
+    int fd = aXSocketFd.fd; // Copy to discard |const| qualifier
+    mPluginXSocketFdDup.forget();
+    mPluginXSocketFdDup.reset(fd);
 #endif
     return true;
 }
 
 void
 PluginModuleParent::NPP_URLRedirectNotify(NPP instance, const char* url,
                                           int32_t status, void* notifyData)
 {
--- a/gfx/src/X11Util.h
+++ b/gfx/src/X11Util.h
@@ -47,16 +47,18 @@
 #elif defined(MOZ_WIDGET_QT)
 #include "gfxQtPlatform.h"
 #undef CursorShape
 #  include <X11/Xlib.h>
 #else
 #  error Unknown toolkit
 #endif 
 
+#include "mozilla/Scoped.h"
+
 #include "gfxCore.h"
 #include "nsDebug.h"
 
 namespace mozilla {
 
 /**
  * Return the default X Display created and used by the UI toolkit.
  */
@@ -80,48 +82,24 @@ DefaultXDisplay()
 bool
 XVisualIDToInfo(Display* aDisplay, VisualID aVisualID,
                 Visual** aVisual, unsigned int* aDepth);
 
 /**
  * Invoke XFree() on a pointer to memory allocated by Xlib (if the
  * pointer is nonnull) when this class goes out of scope.
  */
-template<typename T>
-struct ScopedXFree
+template <typename T>
+struct ScopedXFreePtrTraits
 {
-  ScopedXFree() : mPtr(NULL) {}
-  ScopedXFree(T* aPtr) : mPtr(aPtr) {}
-
-  ~ScopedXFree() { Assign(NULL); }
-
-  ScopedXFree& operator=(T* aPtr) { Assign(aPtr); return *this; }
-
-  operator T*() const { return get(); }
-  T* operator->() const { return get(); }
-  T* get() const { return mPtr; }
-
-private:
-  void Assign(T* aPtr)
-  {
-    NS_ASSERTION(!mPtr || mPtr != aPtr, "double-XFree() imminent");
-
-    if (mPtr)
-      XFree(mPtr);
-    mPtr = aPtr;
-  }
-
-  T* mPtr;
-
-  // disable these
-  ScopedXFree(const ScopedXFree&);
-  ScopedXFree& operator=(const ScopedXFree&);
-  static void* operator new (size_t);
-  static void operator delete (void*);
+  typedef T *type;
+  static T *empty() { return NULL; }
+  static void release(T *ptr) { if (ptr!=NULL) XFree(ptr); }
 };
+SCOPED_TEMPLATE(ScopedXFree, ScopedXFreePtrTraits);
 
 /**
  * On construction, set a graceful X error handler that doesn't crash the application and records X errors.
  * On destruction, restore the X error handler to what it was before construction.
  * 
  * The SyncAndGetError() method allows to know whether a X error occurred, optionally allows to get the full XErrorEvent,
  * and resets the recorded X error state so that a single X error will be reported only once.
  *
--- a/ipc/ril/Ril.cpp
+++ b/ipc/ril/Ril.cpp
@@ -169,17 +169,17 @@ void RilReconnectTask::Run() {
     Enqueue(1000);
 }
 
 class RilWriteTask : public Task {
     virtual void Run();
 };
 
 void RilWriteTask::Run() {
-    sClient->OnFileCanWriteWithoutBlocking(sClient->mSocket.mFd);
+    sClient->OnFileCanWriteWithoutBlocking(sClient->mSocket.rwget());
 }
 
 static void
 ConnectToRil(Monitor* aMonitor, bool* aSuccess)
 {
     MOZ_ASSERT(!sClient);
 
     sClient = new RilClient();
@@ -200,63 +200,63 @@ RilClient::OpenSocket()
     // before we see how this works on the phone.
     struct sockaddr_un addr;
     socklen_t alen;
     size_t namelen;
     int err;
     memset(&addr, 0, sizeof(addr));
     strcpy(addr.sun_path, RIL_SOCKET_NAME);
     addr.sun_family = AF_LOCAL;
-    mSocket.mFd = socket(AF_LOCAL, SOCK_STREAM, 0);
+    mSocket.reset(socket(AF_LOCAL, SOCK_STREAM, 0));
     alen = strlen(RIL_SOCKET_NAME) + offsetof(struct sockaddr_un, sun_path) + 1;
 #else
     struct hostent *hp;
     struct sockaddr_in addr;
     socklen_t alen;
 
     hp = gethostbyname("localhost");
     if (hp == 0) return false;
 
     memset(&addr, 0, sizeof(addr));
     addr.sin_family = hp->h_addrtype;
     addr.sin_port = htons(RIL_TEST_PORT);
     memcpy(&addr.sin_addr, hp->h_addr, hp->h_length);
-    mSocket.mFd = socket(hp->h_addrtype, SOCK_STREAM, 0);
+    mSocket.reset(socket(hp->h_addrtype, SOCK_STREAM, 0));
     alen = sizeof(addr);
 #endif
 
-    if (mSocket.mFd < 0) {
+    if (mSocket.get() < 0) {
         LOG("Cannot create socket for RIL!\n");
         return false;
     }
 
-    if (connect(mSocket.mFd, (struct sockaddr *) &addr, alen) < 0) {
+    iif (connect(mSocket.get(), (struct sockaddr *) &addr, alen) < 0) {
 #if defined(MOZ_WIDGET_GONK)
         LOG("Cannot open socket for RIL!\n");
 #endif
-        close(mSocket.mFd);
+        mSocket.dispose();
         return false;
     }
 
     // Set close-on-exec bit.
-    int flags = fcntl(mSocket.mFd, F_GETFD);
+    int flags = fcntl(mSocket.get(), F_GETFD);
     if (-1 == flags) {
         return false;
     }
 
     flags |= FD_CLOEXEC;
-    if (-1 == fcntl(mSocket.mFd, F_SETFD, flags)) {
+    if (-1 == fcntl(mSocket.get(), F_SETFD, flags)) {
         return false;
     }
 
     // Select non-blocking IO.
-    if (-1 == fcntl(mSocket.mFd, F_SETFL, O_NONBLOCK)) {
+    if (-1 == fcntl(mSocket.get(), F_SETFL, O_NONBLOCK)) {
         return false;
     }
-    if (!mIOLoop->WatchFileDescriptor(mSocket.mFd,
+    if (!mIOLoop->WatchFileDescriptor(mSocket.get(),
                                       true,
                                       MessageLoopForIO::WATCH_READ,
                                       &mReadWatcher,
                                       this)) {
         return false;
     }
     LOG("Socket open for RIL\n");
     return true;
@@ -269,17 +269,17 @@ RilClient::OnFileCanReadWithoutBlocking(
     //
     //   - mIncoming is completely read
     //     If so, sConsumer->MessageReceived(mIncoming.forget())
     //
     //   - mIncoming isn't completely read, but there's no more
     //     data available on the socket
     //     If so, break;
 
-    MOZ_ASSERT(fd == mSocket.mFd);
+    MOZ_ASSERT(fd == mSocket.get());
     while (true) {
         if (!mIncoming) {
             mIncoming = new RilRawData();
             ssize_t ret = read(fd, mIncoming->mData, RilRawData::MAX_DATA_SIZE);
             if (ret <= 0) {
                 if (ret == -1) {
                     if (errno == EINTR) {
                         continue; // retry system call when interrupted
@@ -290,17 +290,17 @@ RilClient::OnFileCanReadWithoutBlocking(
                     // else fall through to error handling on other errno's
                 }
                 LOG("Cannot read from network, error %d\n", ret);
                 // At this point, assume that we can't actually access
                 // the socket anymore, and start a reconnect loop.
                 mIncoming.forget();
                 mReadWatcher.StopWatchingFileDescriptor();
                 mWriteWatcher.StopWatchingFileDescriptor();
-                close(mSocket.mFd);
+                close(mSocket.get());
                 RilReconnectTask::Enqueue();
                 return;
             }
             mIncoming->mSize = ret;
             sConsumer->MessageReceived(mIncoming.forget());
             if (ret < ssize_t(RilRawData::MAX_DATA_SIZE)) {
                 return;
             }
@@ -313,17 +313,17 @@ RilClient::OnFileCanWriteWithoutBlocking
 {
     // Try to write the bytes of mCurrentRilRawData.  If all were written, continue.
     //
     // Otherwise, save the byte position of the next byte to write
     // within mCurrentRilRawData, and request another write when the
     // system won't block.
     //
 
-    MOZ_ASSERT(fd == mSocket.mFd);
+    MOZ_ASSERT(fd == mSocket.get());
 
     while (!mOutgoingQ.empty() || mCurrentRilRawData != NULL) {
         if(!mCurrentRilRawData) {
             mCurrentRilRawData = mOutgoingQ.front();
             mOutgoingQ.pop();
             mCurrentWriteOffset = 0;
         }
         const uint8_t *toWrite;
--- a/modules/libjar/nsZipArchive.cpp
+++ b/modules/libjar/nsZipArchive.cpp
@@ -135,17 +135,17 @@ nsZipHandle::nsZipHandle()
 }
 
 NS_IMPL_THREADSAFE_ADDREF(nsZipHandle)
 NS_IMPL_THREADSAFE_RELEASE(nsZipHandle)
 
 nsresult nsZipHandle::Init(nsILocalFile *file, nsZipHandle **ret)
 {
   mozilla::AutoFDClose fd;
-  nsresult rv = file->OpenNSPRFileDesc(PR_RDONLY, 0000, &fd);
+  nsresult rv = file->OpenNSPRFileDesc(PR_RDONLY, 0000, &fd.rwget());
   if (NS_FAILED(rv))
     return rv;
 
   PRInt64 size = PR_Available64(fd);
   if (size >= PR_INT32_MAX)
     return NS_ERROR_FILE_TOO_BIG;
 
   PRFileMap *map = PR_CreateFileMap(fd, size, PR_PROT_READONLY);
--- a/toolkit/components/telemetry/Telemetry.cpp
+++ b/toolkit/components/telemetry/Telemetry.cpp
@@ -1347,17 +1347,17 @@ TelemetrySessionData::LoadFromDisk(nsIFi
   *ptr = nsnull;
   nsresult rv;
   nsCOMPtr<nsILocalFile> f(do_QueryInterface(file, &rv));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   AutoFDClose fd;
-  rv = f->OpenNSPRFileDesc(PR_RDONLY, 0, &fd);
+  rv = f->OpenNSPRFileDesc(PR_RDONLY, 0, &fd.rwget());
   if (NS_FAILED(rv)) {
     return NS_ERROR_FAILURE;
   }
 
   // If there's not even enough data to read the header for the pickle,
   // don't bother.  Conveniently, this handles the error case as well.
   PRInt32 size = PR_Available(fd);
   if (size < static_cast<PRInt32>(sizeof(Pickle::Header))) {
@@ -1434,17 +1434,17 @@ TelemetrySessionData::SaveToDisk(nsIFile
 {
   nsresult rv;
   nsCOMPtr<nsILocalFile> f(do_QueryInterface(file, &rv));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   AutoFDClose fd;
-  rv = f->OpenNSPRFileDesc(PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0600, &fd);
+  rv = f->OpenNSPRFileDesc(PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE, 0600, &fd.rwget());
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   Pickle pickle;
   if (!pickle.WriteUInt32(sVersion)) {
     return NS_ERROR_FAILURE;
   }
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
@@ -493,17 +493,18 @@ nsUrlClassifierPrefixSet::LoadFromFile(n
 {
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_PS_FILELOAD_TIME> timer;
 
   nsresult rv;
   nsCOMPtr<nsILocalFile> file(do_QueryInterface(aFile, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
 
   AutoFDClose fileFd;
-  rv = file->OpenNSPRFileDesc(PR_RDONLY | nsILocalFile::OS_READAHEAD, 0, &fileFd);
+  rv = file->OpenNSPRFileDesc(PR_RDONLY | nsILocalFile::OS_READAHEAD,
+                              0, &fileFd.rwget());
   NS_ENSURE_SUCCESS(rv, rv);
 
   return LoadFromFd(fileFd);
 }
 
 nsresult
 nsUrlClassifierPrefixSet::StoreToFd(AutoFDClose& fileFd)
 {
@@ -551,15 +552,15 @@ nsUrlClassifierPrefixSet::StoreToFile(ns
   }
 
   nsresult rv;
   nsCOMPtr<nsILocalFile> file(do_QueryInterface(aFile, &rv));
   NS_ENSURE_SUCCESS(rv, rv);
 
   AutoFDClose fileFd;
   rv = file->OpenNSPRFileDesc(PR_RDWR | PR_TRUNCATE | PR_CREATE_FILE,
-                              0644, &fileFd);
+                              0644, &fileFd.rwget());
   NS_ENSURE_SUCCESS(rv, rv);
 
   MutexAutoLock lock(mPrefixSetLock);
 
   return StoreToFd(fileFd);
 }
--- a/widget/gonk/Framebuffer.cpp
+++ b/widget/gonk/Framebuffer.cpp
@@ -74,62 +74,62 @@ typedef vector<nsRefPtr<gfxImageSurface>
 BufferVector* sBuffers;
 
 BufferVector& Buffers() { return *sBuffers; }
 
 bool
 SetGraphicsMode()
 {
     ScopedClose fd(open("/dev/tty0", O_RDWR | O_SYNC));
-    if (0 > fd.mFd) {
+    if (0 > fd.get()) {
         // This is non-fatal; post-Cupcake kernels don't have tty0.
         LOG("No /dev/tty0?");
-    } else if (ioctl(fd.mFd, KDSETMODE, (void*) KD_GRAPHICS)) {
+    } else if (ioctl(fd.get(), KDSETMODE, (void*) KD_GRAPHICS)) {
         LOG("Error setting graphics mode on /dev/tty0");
         return false;
     }
     return true;
 }
 
 bool
 Open(nsIntSize* aScreenSize)
 {
     if (0 <= sFd)
         return true;
 
     if (!SetGraphicsMode())
         return false;
 
     ScopedClose fd(open("/dev/graphics/fb0", O_RDWR));
-    if (0 > fd.mFd) {
+    if (0 > fd.get()) {
         LOG("Error opening framebuffer device");
         return false;
     }
 
     struct fb_fix_screeninfo fi;
-    if (0 > ioctl(fd.mFd, FBIOGET_FSCREENINFO, &fi)) {
+    if (0 > ioctl(fd.get(), FBIOGET_FSCREENINFO, &fi)) {
         LOG("Error getting fixed screeninfo");
         return false;
     }
 
-    if (0 > ioctl(fd.mFd, FBIOGET_VSCREENINFO, &sVi)) {
+    if (0 > ioctl(fd.get(), FBIOGET_VSCREENINFO, &sVi)) {
         LOG("Error getting variable screeninfo");
         return false;
     }
 
     sMappedSize = fi.smem_len;
     void* mem = mmap(0, sMappedSize, PROT_READ | PROT_WRITE, MAP_SHARED,
-                     fd.mFd, 0);
+                     fd.rwget(), 0);
     if (MAP_FAILED == mem) {
         LOG("Error mmap'ing framebuffer");
         return false;
     }
 
-    sFd = fd.mFd;
-    fd.mFd = -1;
+    sFd = fd.get();
+    fd.forget();
 
     // The android porting doc requires a /dev/graphics/fb0 device
     // that's double buffered with r5g6b5 format.  Hence the
     // hard-coded numbers here.
     gfxASurface::gfxImageFormat format = gfxASurface::ImageFormatRGB16_565;
     int bytesPerPixel = gfxASurface::BytePerPixelFromFormat(format);
     gfxIntSize size(sVi.xres, sVi.yres);
     long stride = size.width * bytesPerPixel;
@@ -150,22 +150,22 @@ Open(nsIntSize* aScreenSize)
 }
 
 bool
 GetSize(nsIntSize *aScreenSize) {
     if (0 <= sFd)
         return true;
 
     ScopedClose fd(open("/dev/graphics/fb0", O_RDWR));
-    if (0 > fd.mFd) {
+    if (0 > fd.get()) {
         LOG("Error opening framebuffer device");
         return false;
     }
 
-    if (0 > ioctl(fd.mFd, FBIOGET_VSCREENINFO, &sVi)) {
+    if (0 > ioctl(fd.get(), FBIOGET_VSCREENINFO, &sVi)) {
         LOG("Error getting variable screeninfo");
         return false;
     }
 
     *aScreenSize = gfxIntSize(sVi.xres, sVi.yres);
     return true;
 }
 
--- a/widget/gtk2/nsSound.cpp
+++ b/widget/gtk2/nsSound.cpp
@@ -288,17 +288,18 @@ NS_IMETHODIMP nsSound::OnStreamComplete(
     rv = tmpFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, PR_IRUSR | PR_IWUSR);
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     ScopedCanberraFile canberraFile(tmpFile);
 
     mozilla::AutoFDClose fd;
-    rv = canberraFile->OpenNSPRFileDesc(PR_WRONLY, PR_IRUSR | PR_IWUSR, &fd);
+    rv = canberraFile->OpenNSPRFileDesc(PR_WRONLY, PR_IRUSR | PR_IWUSR,
+                                        &fd.rwget());
     if (NS_FAILED(rv)) {
         return rv;
     }
 
     // XXX: Should we do this on another thread?
     PRUint32 length = dataLen;
     while (length > 0) {
         PRInt32 amount = PR_Write(fd, data, length);
--- a/xpcom/build/FileLocation.cpp
+++ b/xpcom/build/FileLocation.cpp
@@ -136,17 +136,17 @@ FileLocation::Equals(const FileLocation 
   }
   return a->Equals(*b);
 }
 
 nsresult
 FileLocation::GetData(Data &data)
 {
   if (!IsZip()) {
-    return mBaseFile->OpenNSPRFileDesc(PR_RDONLY, 0444, &data.mFd);
+    return mBaseFile->OpenNSPRFileDesc(PR_RDONLY, 0444, &data.mFd.rwget());
   }
   data.mZip = mBaseZip;
   if (!data.mZip) {
     data.mZip = new nsZipArchive();
     data.mZip->OpenArchive(mBaseFile);
   }
   data.mItem = data.mZip->GetItem(mPath.get());
   if (data.mItem)
--- a/xpcom/glue/FileUtils.h
+++ b/xpcom/glue/FileUtils.h
@@ -42,53 +42,53 @@
 
 #if defined(XP_UNIX) || defined(XP_OS2)
 # include <unistd.h>
 #elif defined(XP_WIN)
 # include <io.h>
 #endif
 #include "prio.h"
 
+#include "mozilla/Scoped.h"
+
 namespace mozilla {
 
 /**
  * AutoFDClose is a RAII wrapper for PRFileDesc.
+ *
+ * Instances |PR_Close| their fds when they go out of scope.
  **/
-class AutoFDClose
+struct ScopedClosePRFDTraits
 {
-public:
-  AutoFDClose(PRFileDesc* fd = nsnull) : mFD(fd) { }
-  ~AutoFDClose() { if (mFD) PR_Close(mFD); }
-
-  PRFileDesc* operator= (PRFileDesc *fd) {
-    if (mFD) PR_Close(mFD);
-    mFD = fd;
-    return fd;
+  typedef PRFileDesc* type;
+  static type empty() { return NULL; }
+  static void release(type fd) {
+    if (fd != NULL) {
+      PR_Close(fd);
+    }
   }
-
-  operator PRFileDesc* () { return mFD; }
-  PRFileDesc** operator &() { *this = nsnull; return &mFD; }
-
-private:
-  PRFileDesc *mFD;
 };
+typedef Scoped<ScopedClosePRFDTraits> AutoFDClose;
 
 /**
- * Instances close() their fds when they go out of scope.
+ * ScopedCloseFD is a RAII wrapper for POSIX file descriptors
+ *
+ * Instances |close()| their fds when they go out of scope.
  */
-struct ScopedClose
+struct ScopedCloseFDTraits
 {
-  ScopedClose(int aFd=-1) : mFd(aFd) {}
-  ~ScopedClose() {
-    if (0 <= mFd) {
-      close(mFd);
+  typedef int type;
+  static type empty() { return -1; }
+  static void release(type fd) {
+    if (fd != -1) {
+      close(fd);
     }
   }
-  int mFd;
 };
+typedef Scoped<ScopedCloseFDTraits> ScopedClose;
 
 /**
  * Fallocate efficiently and continuously allocates files via fallocate-type APIs.
  * This is useful for avoiding fragmentation.
  * On sucess the file be padded with zeros to grow to aLength.
  *
  * @param aFD file descriptor.
  * @param aLength length of file to grow to.