OSX memory_maps() - add error handling

1 files changed, 25 insertions, 24 deletions

diff --git a/psutil/_psutil_osx.c b/psutil/_psutil_osx.c
index ed4e60d3..02b2172d 100644
--- a/psutil/_psutil_osx.c
+++ b/psutil/_psutil_osx.c
@@ -344,6 +344,8 @@ psutil_proc_environ(PyObject *self, PyObject *args) {
 /*
  * Return a list of tuples for every process memory maps.
  * 'procstat' cmdline utility has been used as an example.
+ * This will fail for all processes except os.getpid(),
+ * even as root.
  */
 static PyObject *
 psutil_proc_memory_maps(PyObject *self, PyObject *args) {
@@ -351,40 +353,40 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
     char addr_str[34];
     char perms[8];
     int pagesize = getpagesize();
+    int iteration = 0;
     long pid;
-    kern_return_t err = KERN_SUCCESS;
+    kern_return_t kr;
     mach_port_t task = MACH_PORT_NULL;
     uint32_t depth = 1;
     vm_address_t address = 0;
     vm_size_t size = 0;
-
+    struct vm_region_submap_info_64 info;
+    mach_msg_type_number_t count = VM_REGION_SUBMAP_INFO_COUNT_64;
     PyObject *py_tuple = NULL;
     PyObject *py_path = NULL;
     PyObject *py_list = PyList_New(0);
 
     if (py_list == NULL)
         return NULL;
-
     if (! PyArg_ParseTuple(args, "l", &pid))
         goto error;
-
     if (psutil_task_for_pid(pid, &task) != 0)
         goto error;
 
     while (1) {
+        iteration += 1;
         py_tuple = NULL;
-        struct vm_region_submap_info_64 info;
-        mach_msg_type_number_t count = VM_REGION_SUBMAP_INFO_COUNT_64;
-
-        err = vm_region_recurse_64(task, &address, &size, &depth,
+        kr = vm_region_recurse_64(task, &address, &size, &depth,
                                    (vm_region_info_64_t)&info, &count);
-        if (err == KERN_INVALID_ADDRESS) {
-            // TODO temporary
-            psutil_debug("vm_region_recurse_64 returned KERN_INVALID_ADDRESS");
-            break;
-        }
-        if (err != KERN_SUCCESS) {
-            psutil_debug("vm_region_recurse_64 returned !=  KERN_SUCCESS");
+        if (kr != KERN_SUCCESS) {
+            if ((kr == KERN_INVALID_ADDRESS) && (iteration > 1)) {
+                break;  // no more regions to read
+            }
+            PyErr_Format(
+                PyExc_RuntimeError,
+                "vm_region_recurse_64() failed: %s",
+                mach_error_string(kr));
+            goto error;
         }
 
         if (info.is_submap) {
@@ -408,14 +410,13 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
                     (info.max_protection & VM_PROT_WRITE) ? 'w' : '-',
                     (info.max_protection & VM_PROT_EXECUTE) ? 'x' : '-');
 
-            // proc_regionfilename() return value seems meaningless
-            // so we do what we can in order to not continue in case
-            // of error.
+            // proc_regionfilename() is undocumented but from its source
+            // code we can determine that it sets errno on error and
+            // return length of path.
             errno = 0;
             proc_regionfilename((pid_t)pid, address, buf, sizeof(buf));
-            if ((errno != 0) || ((sizeof(buf)) <= 0)) {
-                // TODO temporary
-                psutil_debug("proc_regionfilename() failed");
+            if (errno != 0) {
+                psutil_debug("proc_regionfilename() failed errno=%i", errno);
                 psutil_raise_for_pid(pid, "proc_regionfilename()");
                 goto error;
             }
@@ -477,10 +478,10 @@ psutil_proc_memory_maps(PyObject *self, PyObject *args) {
                 goto error;
             Py_DECREF(py_tuple);
             Py_DECREF(py_path);
-        }
 
-        // increment address for the next map/file
-        address += size;
+            // increment address for the next map/file
+            address += size;
+        }
     }
 
     if (task != MACH_PORT_NULL)