lots of docs + small tweaks for consistency

include/mach_detours.h

@@ -19,71 +19,175 @@ typedef void* detour_func_t;
 #define	detour_err_too_small	(err_local | 3)
 #define	detour_err_too_large	(err_local | 4)
 
-/// Begin a new transaction on the current thread
+/// @brief Begin a new transaction on the current thread
 ///
-/// @note This will mark all trampoline regions as writable and not executable. Until the transaction is committed or
+/// If there is no ongoing transaction, marks the calling thread as owning the transaction.
-///       aborted, no thread can execute the trampoline code. If any threads might be doing that, consider using
+/// This will be active until it is either committed or aborted.
-///       detour_transaction_begin_managed instead, to ensure all threads are suspended.
+///
+/// This will also mark all existing trampoline regions (from a previous transaction) as writable (and not executable).
+/// Therefore, no thread can execute code in the trampoline regions. Consider using `detour_transaction_begin_managed`
+/// to ensure all threads are suspended.
+///
+/// @returns `err_none` on success, else:<br/>
+///     * `detour_err_in_progress` if there is an ongoing transaction on any thread.
+///     * If `mach_vm_protect` fails for any trampoline region, forwards the error code from the last attempt (though
+///       it continues with the remaining regions)
 mach_error_t detour_transaction_begin();
 
-/// Begin a new transaction on the current thread and immediately manage all (other) threads
+/// @brief Begin a new transaction on the current thread and immediately manage all (other) threads
 /// @see `detour_transaction_begin`
 /// @see `detour_manage_all_threads`
+///
+/// @returns `err_none` on success, else:<br/>
+///     * `detour_err_in_progress` if there is an ongoing transaction on any thread.
+///     * If `detour_manage_all_threads` fails, forwards its error code
+///     * If `mach_vm_protect` fails for any trampoline region, forwards the error code from the last attempt (though
+///       it continues with the remaining regions)
 mach_error_t detour_transaction_begin_managed();
 
-/// @note Requires an active transaction started on the current thread. If there is no transaction active at all, the
+/// @brief Abort the current transaction, if any.
-///       function has no effect.
+///
+/// * Restores all page permissions on target function code
+/// * Discards all scheduled attach and detach operations in the current transaction
+/// * Restores all page permissions on trampoline regions
+/// * Resumes all threads previously suspended by `detour_manage_thread`.
+/// * Removes the current transaction
+///
+/// @returns `err_none` on success, else:<br/>
+///     * `err_none` if there is no current transaction.
+///     * `detour_err_wrong_thread` if the calling thread is not the owner of the transaction. See `detour_transaction_begin`.
+/// @note Can be called when there is no ongoing transaction. Then this is a no-op.
 mach_error_t detour_transaction_abort();
 
-/// @see `detour_transaction_commit_ex`
+/// @brief Commits the current transaction
+///
+/// Applies all pending attach/detach operations from the current transaction:<br/>
+/// * Actually modify the target function to either insert or remove the detour
+/// * Update the `inout_pointer` passed to attach/detach, so it points to the original function (trampoline after an
+///   attach operation, original function address after a detach operation)
+/// * Update the instruction pointer of all managed threads, in case they were inside the modified memory regions
+/// * Restore all page permissions (of target functions and trampolines)
+/// * Potentially frees any unused trampoline regions (see also `detour_set_retain_regions`)
+/// * Resumes all threads previously suspended by `detour_manage_thread`.
+/// * Removes the current transaction
+///
+/// @returns `err_none` on success, else:<br/>
+///     * `detour_err_wrong_thread` if the calling thread is not the owner of the transaction, or if there is no
+///       transaction. See `detour_transaction_begin`.
+///     * If a previous `detour_attach` or `detour_detach` operation failed, returns the error code from that call.
+///
+/// @note If there is a pending error from a failed attach/detach operation, instead aborts the transaction and returns
+/// that pending error code.
 mach_error_t detour_transaction_commit();
 
-/// @note Requires an active transaction started on the current thread.
+/// @brief Same as `detour_transaction_commit`, but in case of an error returns the first failed attach/detach operation's
+/// target function.
+///
+/// @param out_failed_target in case of an error, will receive the inout_pointer associated with the failed operation
+/// @returns same as detour_transaction_commit
 mach_error_t detour_transaction_commit_ex(detour_func_t** out_failed_target);
 
 
-/// Suspends the given thread and marks it as pending. It will be resumed when the transaction is committed or aborted.
+/// @brief Suspends the given thread and marks it as pending. It will be resumed when the transaction is committed or aborted.
+///
+/// @param thread the thread to manage
+/// @returns `err_none` on success, else:<br/>
+///     * If a previous `detour_attach` or `detour_detach` operation failed, returns the error code from that call.
+///     * `detour_err_wrong_thread` if the calling thread is not the owner of the transaction, or if there is no
+///       transaction. See `detour_transaction_begin`.
+///     * `err_none` if the given thread is the calling thread
+///     * `KERN_RESOURCE_SHORTAGE` if `calloc` failed for the structure keeping track of the thread
+///     * If `thread_suspend` fails, forwards its error code
+///
 /// @note Requires an active transaction started on the current thread.
 /// @note Calling this function with the transaction thread has no effect.
 mach_error_t detour_manage_thread(thread_t thread);
 
-/// Manages all threads via `detour_manage_thread`
+/// @brief Manages all threads via `detour_manage_thread`
-/// @see `detour_manage_thread`
+///
+/// @returns `err_none` on success, else:<br/>
+///     * If `task_threads` fails, forwards its error code
+///     * If `detour_manage_thread` fails for any thread, forwards the error code from the last attempt (though
+///       it continues with the remaining threads)
+///
 /// @note Requires an active transaction started on the current thread.
 mach_error_t detour_manage_all_threads();
 
 
-/// @see `detour_attach_ex`
+/// @brief Schedules installing a detour
-mach_error_t detour_attach(detour_func_t* inout_pointer, detour_func_t detour);
+///
-
+/// @param inout_pointer pointer to a value that points at the target function<br/>
-/// @note Requires an active transaction started on the current thread.
+///                      When the transaction is committed, the target value (`*inout_pointer`) will be assigned the
+///                      address of the trampoline (the way to invoke the original function while the detour is in
+///                      place).
+/// @param detour detour function to insert
+/// @returns `err_none` on success, else:<br/>
+///     * `detour_err_wrong_thread` if the calling thread is not the owner of the transaction, or if there is no
+///       transaction. See `detour_transaction_begin`.
+///     * If a previous `detour_attach` or `detour_detach` operation failed, returns the error code from that call.
+///     * `KERN_INVALID_ARGUMENT` if any of `detour`, `inout_pointer`, or `*inout_pointer` is nullptr
+///     * `detour_err_too_small` if the target function only points to the detour, or if the target function is too
+///       short to insert a detour
+///     * `detour_err_too_large` if the instructions copied from the original function are too large to fit into the
+///       trampoline. This is likely a bug in mach_detours.
+///     * `KERN_RESOURCE_SHORTAGE` if `calloc` failed for the structure keeping track of the operation or the trampoline
+///     * If `mach_vm_region_recurse` fails on the target address, forwards its error code
+///     * If `mach_vm_protect` fails on the target address, forwards its error code
+///
 /// @note After calling this function, the memory page containing the target function (*inout_pointer) is no longer
 ///       executable, until you call either commit or abort the current transaction. Due to this, you might run into
 ///       EXC_BAD_ACCESS when hooking into local functions in the same executable as the calling function.
 ///       You can use detour_attach_and_commit/detour_attach_and_commit_ex instead in this case.<br/>
 ///       Similarly, no caller can execute the target function (whether local or not), including the transaction thread.
 ///       Make sure to detour_manage_thread/detour_manage_all_threads in this case.
+/// @note Requires an active transaction started on the current thread.
+mach_error_t detour_attach(detour_func_t* inout_pointer, detour_func_t detour);
+
+/// @brief Same as `detour_attach`, but optionally returns the trampoline, target and detour function pointers
+///
+/// @param inout_pointer same as in `detour_attach`
+/// @param detour same as in `detour_attach`
+/// @param out_real_trampoline on success, if not null will get the trampoline address
+/// @param out_real_target on success, if not null will get the target function address
+/// @param out_real_detour on success, if not null will get the detour function address
 mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour, detour_func_t* out_real_trampoline, detour_func_t* out_real_target, detour_func_t* out_real_detour);
 
-/// @note Requires an active transaction started on the current thread.
+/// @brief Schedules removing a detour
+///
+/// @param inout_pointer same inout_pointer as passed to detour_attach<br/>
+///                      When the transaction is committed, the target value (`*inout_pointer`) will be restored to the
+///                      address of the target function once more, since the trampoline will be deallocated.
+/// @param detour same detour as passed to detour_attach
+/// @returns `err_none` on success, else:<br/>
+///     * `detour_err_wrong_thread` if the calling thread is not the owner of the transaction, or if there is no
+///       transaction. See `detour_transaction_begin`.
+///     * If a previous `detour_attach` or `detour_detach` operation failed, returns the error code from that call.
+///     * `KERN_INVALID_ARGUMENT` if any of `detour`, `inout_pointer`, or `*inout_pointer` is nullptr
+///     * `KERN_FAILURE` if `inout_pointer` or `detour` are not a valid combination, or that detour is not yet or no
+///        longer committed.
+///     * `KERN_RESOURCE_SHORTAGE` if `calloc` failed for the structure keeping track of the operation
+///     * If `mach_vm_region_recurse` fails on the target address, forwards its error code
+///     * If `mach_vm_protect` fails on the target address, forwards its error code
+///
 /// @note After calling this function, the memory page containing the target function (*inout_pointer) is no longer
 ///       executable, until you call either commit or abort the current transaction. Due to this, you might run into
 ///       EXC_BAD_ACCESS when hooking into local functions in the same executable as the calling function.
 ///       You can use detour_detach_and_commit instead in this case.
+/// @note Requires an active transaction started on the current thread.
 mach_error_t detour_detach(detour_func_t* inout_pointer, detour_func_t detour);
 
 
-/// Same as calling `detour_attach(inout_pointer, detour)` and `detour_transaction_commit()`
+/// @brief Same as calling `detour_attach(inout_pointer, detour)` and `detour_transaction_commit()`
 /// @see `detour_attach`
 /// @see `detour_transaction_commit`
 mach_error_t detour_attach_and_commit(detour_func_t* inout_pointer, detour_func_t detour);
 
-/// Same as calling `detour_attach_ex(inout_pointer, detour, out_real_trampoline, out_real_target, out_real_detour)` and `detour_transaction_commit()`
+/// @brief Same as calling `detour_attach_ex(inout_pointer, detour, out_real_trampoline, out_real_target, out_real_detour)` and `detour_transaction_commit()`
 /// @see `detour_attach_ex`
 /// @see `detour_transaction_commit`
 mach_error_t detour_attach_and_commit_ex(detour_func_t* inout_pointer, detour_func_t detour, detour_func_t* out_real_trampoline, detour_func_t* out_real_target, detour_func_t* out_real_detour);
 
-/// Same as calling `detour_detach(inout_pointer, detour)` and `detour_transaction_commit()`
+/// @brief Same as calling `detour_detach(inout_pointer, detour)` and `detour_transaction_commit()`
 /// @see `detour_detach`
 /// @see `detour_transaction_commit`
 mach_error_t detour_detach_and_commit(detour_func_t* inout_pointer, detour_func_t detour);

src/mach_detours.c

@@ -582,16 +582,20 @@ mach_error_t detour_manage_all_threads()
     const mach_port_t port = mach_task_self();
     thread_act_array_t threads = nullptr;
     mach_msg_type_number_t numThreads = 0;
-    const kern_return_t result = task_threads(port, &threads, &numThreads);
+    mach_error_t result = task_threads(port, &threads, &numThreads);
     if (result != err_none) {
         return result;
     }
     for (mach_msg_type_number_t i = 0; i < numThreads; i++) {
-        DETOUR_CHECK( detour_manage_thread(threads[i]) );
+        const mach_error_t error = detour_manage_thread(threads[i]);
+        if (error != err_none) {
+            DETOUR_BREAK();
+            result = error;
+        }
     }
 
     DETOUR_CHECK( vm_deallocate(port, (vm_address_t)threads, numThreads * sizeof(*threads)) );
-    return err_none;
+    return result;
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -614,6 +618,17 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour
     if (out_real_detour) {
         *out_real_detour = nullptr;
     }
+
+    if (s_transaction_thread != mach_thread_self()) {
+        return detour_err_wrong_thread;
+    }
+
+    // If any of the pending operations failed, then we don't need to do this.
+    if (s_pending_error != err_none) {
+        DETOUR_TRACE(("pending transaction error=%d\n", s_pending_error));
+        return s_pending_error;
+    }
+
     if (!detour) {
         DETOUR_TRACE(("empty detour\n"));
         return KERN_INVALID_ARGUMENT;
@@ -631,20 +646,6 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour
         return s_pending_error;
     }
 
-    {
-        const thread_t active_thread = s_transaction_thread;
-        if (active_thread != mach_thread_self()) {
-            DETOUR_TRACE(("transaction conflict with thread id=%u\n", active_thread));
-            return detour_err_wrong_thread;
-        }
-    }
-
-    // If any of the pending operations failed, then we don't need to do this.
-    if (s_pending_error != err_none) {
-        DETOUR_TRACE(("pending transaction error=%d\n", s_pending_error));
-        return s_pending_error;
-    }
-
     mach_error_t error = err_none;
     detour_trampoline* trampoline = nullptr;
     detour_operation* op = nullptr;
@@ -959,7 +960,7 @@ mach_error_t detour_detach(detour_func_t* inout_pointer, detour_func_t detour)
     op->next = s_pending_operations_head;
     s_pending_operations_head = op;
 
-    return ERR_SUCCESS;
+    return err_none;
 }
 
 mach_error_t detour_attach_and_commit(detour_func_t* inout_pointer, detour_func_t detour)