From 2908a52bfab6d36c8a5e76c992fe1399d00af52f Mon Sep 17 00:00:00 2001
From: Thiago Canozzo Lahr <tclahr@gmail.com>
Date: Wed, 14 Aug 2024 08:52:57 -0300
Subject: [PATCH] fix: xargs max-procs concurrency

This patch removes xargs max-procs parameter as it was not
properly managing parallel access to shared resources such as
output files for stat and hash collectors.

Fixes #267
---
 CHANGELOG.md                        |  1 -
 lib/find_based_collector.sh         | 12 ++++++------
 lib/parse_command_line_arguments.sh | 16 +---------------
 lib/remove_non_regular_files.sh     |  7 ++++++-
 lib/setup_tools.sh                  |  4 ----
 lib/usage.sh                        |  5 -----
 uac                                 | 13 -------------
 7 files changed, 13 insertions(+), 45 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6220b7f6..9a80ff7d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,7 +19,6 @@
 - The message 'The strings command requires the command line developer tools.' will no longer appear on macOS systems without developer tools installed ([#171](https://github.com/tclahr/uac/issues/171)).
 - Error messages generated by executed commands (stderr) are now recorded in the uac.log file ([#150](https://github.com/tclahr/uac/issues/150)).
 - New '-H/--hash-collected' command line option. Enabling this option will cause UAC to hash all collected files and save the results in a hash file. To accomplish this, all collected data must first be copied to the destination directory. Therefore, ensure you have twice the free space available on the system: once for the collected data and once for the output file. Additionally, note that this process will increase the running time ([#189](https://github.com/tclahr/uac/issues/189)).
-- New '-t/--max-thread' command line option. It can be used to specify the number of files that will be processed in parallel by the 'hash' and 'stat' collectors.
 - You can now validate profiles using the '--validate-profile' command line option.
 
 ### Artifacts
diff --git a/lib/find_based_collector.sh b/lib/find_based_collector.sh
index 707365ef..9c07f78d 100644
--- a/lib/find_based_collector.sh
+++ b/lib/find_based_collector.sh
@@ -169,7 +169,7 @@ _find_based_collector()
 
         if [ -n "${__fc_hashing_tool}" ]; then
           if ${__fc_is_file_list}; then
-            __fc_hash_command="sed 's|.|\\\\&|g' \"${__fc_path}\" | xargs ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__fc_hashing_tool}"
+            __fc_hash_command="sed 's|.|\\\\&|g' \"${__fc_path}\" | xargs ${__fc_hashing_tool}"
           elif ${__UAC_TOOL_FIND_PRINT0_SUPPORT} && ${__UAC_TOOL_XARGS_NULL_DELIMITER_SUPPORT}; then
             __fc_find_command=`_build_find_command \
               "${__fc_path}" \
@@ -185,7 +185,7 @@ _find_based_collector()
               "true" \
               "${__fc_start_date_days}" \
               "${__fc_end_date_days}"`
-            __fc_hash_command="${__fc_find_command} | xargs -0 ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__fc_hashing_tool}"
+            __fc_hash_command="${__fc_find_command} | xargs -0 ${__fc_hashing_tool}"
           else
             __fc_find_command=`_build_find_command \
               "${__fc_path}" \
@@ -201,7 +201,7 @@ _find_based_collector()
               "" \
               "${__fc_start_date_days}" \
               "${__fc_end_date_days}"`
-            __fc_hash_command="${__fc_find_command} | sed 's|.|\\\\&|g' | xargs ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__fc_hashing_tool}"
+            __fc_hash_command="${__fc_find_command} | sed 's|.|\\\\&|g' | xargs ${__fc_hashing_tool}"
           fi
           _verbose_msg "${__UAC_VERBOSE_CMD_PREFIX}${__fc_hash_command}"
           _run_command "${__fc_hash_command}" \
@@ -212,7 +212,7 @@ _find_based_collector()
     "stat")
       if [ -n "${__UAC_TOOL_STAT_BIN}" ]; then
         if ${__fc_is_file_list}; then
-            __fc_stat_command="sed 's|.|\\\\&|g' \"${__fc_path}\" | xargs ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
+            __fc_stat_command="sed 's|.|\\\\&|g' \"${__fc_path}\" | xargs ${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
         elif ${__UAC_TOOL_FIND_PRINT0_SUPPORT} && ${__UAC_TOOL_XARGS_NULL_DELIMITER_SUPPORT}; then
           __fc_find_command=`_build_find_command \
             "${__fc_path}" \
@@ -228,7 +228,7 @@ _find_based_collector()
             "true" \
             "${__fc_start_date_days}" \
             "${__fc_end_date_days}"`
-          __fc_stat_command="${__fc_find_command} | xargs -0 ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
+          __fc_stat_command="${__fc_find_command} | xargs -0 ${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
         else
           __fc_find_command=`_build_find_command \
             "${__fc_path}" \
@@ -244,7 +244,7 @@ _find_based_collector()
             "" \
             "${__fc_start_date_days}" \
             "${__fc_end_date_days}"`
-          __fc_stat_command="${__fc_find_command} | sed 's|.|\\\\&|g' | xargs ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
+          __fc_stat_command="${__fc_find_command} | sed 's|.|\\\\&|g' | xargs ${__UAC_TOOL_STAT_BIN}${__UAC_TOOL_STAT_PARAMS:+ }${__UAC_TOOL_STAT_PARAMS}"
         fi
         _verbose_msg "${__UAC_VERBOSE_CMD_PREFIX}${__fc_stat_command}"
         _run_command "${__fc_stat_command}" \
diff --git a/lib/parse_command_line_arguments.sh b/lib/parse_command_line_arguments.sh
index d3ef306d..d72b83e8 100644
--- a/lib/parse_command_line_arguments.sh
+++ b/lib/parse_command_line_arguments.sh
@@ -129,21 +129,7 @@ _parse_command_line_arguments()
         fi
         ;;
       "-H"|"--hash-collected")
-          __UAC_HASH_COLLECTED=true
-          ;;
-      "-t"|"--max-threads")
-        if [ -n "${2:-}" ]; then
-          if [ "${2}" = "list" ]; then
-            __ua_nproc=`_get_nproc`
-            printf "Number of processing units: %s\n" "${__ua_nproc}"
-            _exit_success
-          fi
-          __UAC_MAX_THREADS="${2}"
-          shift
-        else
-          _error_msg "option '${1}' requires an argument.\nTry 'uac --help' for more information."
-          return 1
-        fi
+        __UAC_HASH_COLLECTED=true
         ;;
       "-u"|"--run-as-non-root")
         __UAC_RUN_AS_NON_ROOT=true
diff --git a/lib/remove_non_regular_files.sh b/lib/remove_non_regular_files.sh
index b3996f4f..226a248d 100644
--- a/lib/remove_non_regular_files.sh
+++ b/lib/remove_non_regular_files.sh
@@ -15,7 +15,12 @@ _remove_non_regular_files()
     return 1
   fi
  
-  __rn_command="sed 's|.|\\\\&|g' \"${__rn_file}\" | xargs ${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}${__UAC_TOOL_XARGS_MAX_PROCS_PARAM:+ }find"
+  if [ ! -s "${__rn_file}" ]; then
+    _log_msg DBG "_remove_non_regular_files: skipping empty file '${__rn_file}'"
+    return 1
+  fi
+
+  __rn_command="sed 's|.|\\\\&|g' \"${__rn_file}\" | xargs find"
 
   _verbose_msg "${__UAC_VERBOSE_CMD_PREFIX}${__rn_command}"
   eval "${__rn_command}" \
diff --git a/lib/setup_tools.sh b/lib/setup_tools.sh
index 73b403e9..8511433b 100644
--- a/lib/setup_tools.sh
+++ b/lib/setup_tools.sh
@@ -21,7 +21,6 @@ _setup_tools()
   __UAC_TOOL_FIND_CTIME_SUPPORT=false
   __UAC_TOOL_FIND_PRINT0_SUPPORT=false
   __UAC_TOOL_XARGS_NULL_DELIMITER_SUPPORT=false
-  __UAC_TOOL_XARGS_MAX_PROCS_PARAM=""
   __UAC_TOOL_STAT_BIN=""
   __UAC_TOOL_STAT_PARAMS=""
   __UAC_TOOL_STAT_BTIME=false
@@ -68,9 +67,6 @@ _setup_tools()
   if echo "uac" | xargs -0 echo >/dev/null; then
     __UAC_TOOL_XARGS_NULL_DELIMITER_SUPPORT=true
   fi
-  if echo "uac" | xargs -P 2 echo >/dev/null; then
-    __UAC_TOOL_XARGS_MAX_PROCS_PARAM="-P ${__UAC_MAX_THREADS}"
-  fi
   
   # check which stat tool and options are available for the target system
   if statx "${__UAC_MOUNT_POINT}" | grep -q -E "\|[0-9]+\|[0-9]+\|[0-9]+$"; then
diff --git a/lib/usage.sh b/lib/usage.sh
index 301bbefa..5bc6f7dd 100644
--- a/lib/usage.sh
+++ b/lib/usage.sh
@@ -62,11 +62,6 @@ Collection Arguments:
                              netscaler, openbsd, solaris
   -H, --hash-collected
                     Hash all collected files.
-  -t, --max-threads THREADS
-                    Specify the number of files that will be processed in
-                    parallel by the hash and stat collectors (default: 2).
-                    Use '--max-threads list' to list the number of processing
-                    units available on the system.
   -u, --run-as-non-root
                     Disable root user check.
                     Note that data collection may be limited.
diff --git a/uac b/uac
index 88f0019a..6ef351e0 100755
--- a/uac
+++ b/uac
@@ -61,7 +61,6 @@ __UAC_HASH_COLLECTED=false
 __UAC_CONFIG_FILE="${__UAC_DIR}/config/uac.conf"
 __UAC_MOUNT_POINT="/"
 __UAC_OPERATING_SYSTEM=""
-__UAC_MAX_THREADS="2"
 __UAC_RUN_AS_NON_ROOT=false
 __UAC_PROCESSING_UNITS=""
 __UAC_HOSTNAME=""
@@ -145,13 +144,6 @@ if [ -z "${__UAC_HOSTNAME}" ]; then
   __UAC_HOSTNAME=`_get_hostname "${__UAC_MOUNT_POINT}"`
 fi
 
-# check if max-threads is a positive integer (greater than zero)
-if _is_digit "${__UAC_MAX_THREADS}" && [ "${__UAC_MAX_THREADS}" -ge 1 ]; then
-    true
-else
-  _exit_fatal "'max-threads' must be a positive integer."
-fi
-
 # check whether is running as root
 if _is_root || ${__UAC_RUN_AS_NON_ROOT}; then
   true
@@ -401,11 +393,6 @@ else
   _log_msg INF "perl command exists: false"
 fi
 _log_msg INF "xargs -0 support: ${__UAC_TOOL_XARGS_NULL_DELIMITER_SUPPORT}"
-if [ -n "${__UAC_TOOL_XARGS_MAX_PROCS_PARAM}" ]; then
-  _log_msg INF "xargs -P support: true"
-else
-  _log_msg INF "xargs -P support: false"
-fi
 _log_msg INF "MD5 hashing tool: ${__UAC_TOOL_MD5_BIN}"
 _log_msg INF "SHA1 hashing tool: ${__UAC_TOOL_SHA1_BIN}"
 _log_msg INF "SHA256 hashing tool: ${__UAC_TOOL_SHA256_BIN}"