From 8389da4c07cf115b1462197581490d291a058265 Mon Sep 17 00:00:00 2001
From: Daniel Nichter <daniel@percona.com>
Date: Mon, 30 Jan 2012 09:22:25 -0700
Subject: [PATCH] Use . instead of source.  Quote func file.  Verify
 --function.  Add option_error() to parse_options.sh.  Update libs in
 pt-stalk.

---
 bin/pt-stalk              | 105 +++++++++++++++++++++++---------------
 lib/bash/parse_options.sh |  18 +++----
 2 files changed, 72 insertions(+), 51 deletions(-)

diff --git a/bin/pt-stalk b/bin/pt-stalk
index 3e70c8a0..80c3484b 100755
--- a/bin/pt-stalk
+++ b/bin/pt-stalk
@@ -133,6 +133,12 @@ usage_or_errors() {
    return 0
 }
 
+option_error() {
+   local err="$1"
+   OPT_ERRS=$(($OPT_ERRS + 1))
+   echo "$err" >&2
+}
+
 parse_options() {
    local file="$1"
    shift
@@ -317,8 +323,7 @@ _parse_command_line() {
       if [ "$next_opt_is_val" ]; then
          next_opt_is_val=""
          if [ $# -eq 0 ] || [ $(expr "$opt" : "-") -eq 1 ]; then
-            OPT_ERRS=$(($OPT_ERRS + 1))
-            echo "$real_opt requires a $required_arg argument" >&2
+            option_error "$real_opt requires a $required_arg argument"
             continue
          fi
          val="$opt"
@@ -353,8 +358,7 @@ _parse_command_line() {
          else
             spec=$(grep "^short form:-$opt\$" "$TMPDIR"/po/* | cut -d ':' -f 1)
             if [ -z "$spec"  ]; then
-               OPT_ERRS=$(($OPT_ERRS + 1))
-               echo "Unknown option: $real_opt" >&2
+               option_error "Unknown option: $real_opt"
                continue
             fi
          fi
@@ -368,8 +372,7 @@ _parse_command_line() {
             fi
          else
             if [ "$val" ]; then
-               OPT_ERRS=$(($OPT_ERRS + 1))
-               echo "Option $real_opt does not take a value" >&2
+               option_error "Option $real_opt does not take a value"
                continue
             fi 
             if [ "$opt_is_negated" ]; then
@@ -473,18 +476,17 @@ _seq() {
 }
 
 _pidof() {
-   local proc="$1"       # process name
-   local pat="${2:-""}"  # pattern in case we must grep for proc
-   
-   local pid=""
+   local cmd="$1"
+   if ! pidof "$cmd" 2>/dev/null; then
+      ps -eo pid,ucomm | awk -v comm="$cmd" '$2 == comm { print $1 }'
+   fi
+}
 
-   [ "$CMD_PIDOF" ] && pid=$(pidof -s "$proc");
-
-   [ -z "$pid" ] && [ "$CMD_PGREP" ] && pid=$(pgrep -o -x "$proc");
-
-   [ -z "$pid" ] && pid=$(ps -eaf | grep "$pat" | grep -v mysqld_safe | awk '{print $2}' | head -n1)
-
-   echo $pid
+_lsof() {
+   local pid="$1"
+   if ! lsof -p $pid 2>/dev/null; then
+      /bin/ls -l /proc/$pid/fd 2>/dev/null
+   fi
 }
 
 # ###########################################################################
@@ -610,7 +612,6 @@ set -u
 
 CMD_GDB="$(which gdb)"
 CMD_IOSTAT="$(which iostat)"
-CMD_LSOF="$(which lsof)"
 CMD_MPSTAT="$(which mpstat)"
 CMD_MYSQL="$(which mysql)"
 CMD_MYSQLADMIN="$(which mysqladmin)"
@@ -628,7 +629,7 @@ collect() {
    local d="$1"  # directory to save results in
    local p="$2"  # prefix for each result file
 
-   local mysqld_pid=$(_pidof mysqld mysql[d]);
+   local mysqld_pid=$(_pidof mysqld | head -n1)
 
    if [ "$CMD_PMAP" -a "$mysqld_pid" ]; then
       if $CMD_PMAP --help 2>&1 | grep -- -x >/dev/null 2>&1 ; then
@@ -658,13 +659,13 @@ collect() {
 
    local tail_error_log_pid=""
    if [ "$mysql_error_log" ]; then
-      echo "The MySQL error log seems to be ${mysql_error_log}"
+      log "The MySQL error log seems to be $mysql_error_log"
       tail -f "$mysql_error_log" >"$d/$p-log_error" &
       tail_error_log_pid=$!
 
       $CMD_MYSQLADMIN $EXT_ARGV debug
    else
-      echo "Could not find the MySQL error log" >&2
+      log "Could not find the MySQL error log"
    fi
 
    local innostat="SHOW /*!40100 ENGINE*/ INNODB STATUS\G"
@@ -686,13 +687,13 @@ collect() {
       fi
    fi
 
-   local have_oprofile="no"
+   local have_oprofile=""
    if [ "$CMD_OPCONTROL" -a "$OPT_COLLECT_OPROFILE" ]; then
       if $CMD_OPCONTROL --init; then
          $CMD_OPCONTROL --start --no-vmlinux
          have_oprofile="yes"
       fi
-   elif [ "$CMD_STRACE" -a "$OPT_COLLECT_STRACE" ]; then
+   elif [ "$CMD_STRACE" -a "$OPT_COLLECT_STRACE" -a "$mysqld_pid" ]; then
       $CMD_STRACE -T -s 0 -f -p $mysqld_pid > "${DEST}/$d-strace" &
       local strace_pid=$!
    fi
@@ -700,9 +701,8 @@ collect() {
    ps -eaf  >> "$d/$p-ps"  &
    top -bn1 >> "$d/$p-top" &
 
-   if [ "$CMD_LSOF" ]; then
-      $CMD_LSOF -nP -p $mysqld_pid -bw >> "$d/$p-lsof" &
-   fi  
+   [ "$mysqld_pid" ] && _lsof $mysqld_pid >> "$d/$p-lsof" &
+
    if [ "$CMD_SYSCTL" ]; then
       $CMD_SYSCTL -a >> "$d/$p-sysctl" &
    fi
@@ -722,14 +722,14 @@ collect() {
    $CMD_MYSQLADMIN $EXT_ARGV ext -i1 -c$OPT_RUN_TIME >>"$d/$p-mysqladmin" &
    local mysqladmin_pid=$!
 
-   local have_lock_waits_table=0
+   local have_lock_waits_table=""
    $CMD_MYSQL $EXT_ARGV -e "SHOW TABLES FROM INFORMATION_SCHEMA" \
       | grep -i "INNODB_LOCK_WAITS" >/dev/null 2>&1
    if [ $? -eq 0 ]; then
-      have_lock_waits_table=1
+      have_lock_waits_table="yes"
    fi
 
-   echo "Loop start: $(date +'TS %s.%N %F %T')"
+   log "Loop start: $(date +'TS %s.%N %F %T')"
    for loopno in $(_seq $OPT_RUN_TIME); do
       disk_space $d > $d/$p-disk-space
       check_disk_space          \
@@ -771,16 +771,23 @@ collect() {
       (echo $ts; $CMD_MYSQL $EXT_ARGV -e "SHOW FULL PROCESSLIST\G") \
          >> "$d/$p-processlist" &
 
-      if [ $have_lock_waits_table -eq 1 ]; then
+      if [ "$have_lock_waits_table" ]; then
          (echo $ts; lock_waits) >>"$d/$p-lock-waits" &
       fi
    done
-   echo "Loop end: $(date +'TS %s.%N %F %T')"
+   log "Loop end: $(date +'TS %s.%N %F %T')"
 
-   if [ "$have_oprofile" = "yes" ]; then
+   if [ "$have_oprofile" ]; then
       $CMD_OPCONTROL --stop
       $CMD_OPCONTROL --dump
-      kill $(pidof oprofiled);  # TODO: what if system doesn't have pidof?
+
+      local oprofiled_pid=$(_pidof oprofiled)
+      if [ "$oprofiled_pid" ]; then
+         kill $oprofiled_pid
+      else
+         warn "Cannot kill oprofiled because its PID cannot be determined"
+      fi
+
       $CMD_OPCONTROL --save=pt_collect_$p
 
       local mysqld_path=$(which mysqld);
@@ -793,7 +800,7 @@ collect() {
             "$mysqld_path"        \
             > "$d/$p-opreport"
       else
-         echo "oprofile data saved to pt_collect_$p; you should be able"      \
+         log "oprofile data saved to pt_collect_$p; you should be able"       \
               "to get a report by running something like 'opreport"           \
               "--demangle=smart --symbols --merge tgid session:pt_collect_$p" \
               "/path/to/mysqld'"                                              \
@@ -803,7 +810,7 @@ collect() {
       kill -s 2 $strace_pid
       sleep 1
       kill -s 15 $strace_pid
-      kill -s 18 $mysqld_pid
+      [ "$mysqld_pid" ] && kill -s 18 $mysqld_pid
    fi
 
    $CMD_MYSQL $EXT_ARGV -e "$innostat" >> "$d/$p-innodbstatus2" &
@@ -829,7 +836,7 @@ open_tables() {
    if [ -n "$open_tables" -a $open_tables -le 1000 ]; then
       $CMD_MYSQL $EXT_ARGV -e 'SHOW OPEN TABLES' &
    else
-      echo "Too many open tables: $open_tables"
+      log "Too many open tables: $open_tables"
    fi
 }
 
@@ -873,6 +880,7 @@ lock_waits() {
 # ###########################################################################
 # Global variables
 # ###########################################################################
+TRIGGER_FUNCTION=""
 RAN_WITH=""
 EXIT_REASON=""
 TOOL="pt-stalk"
@@ -928,12 +936,21 @@ grep_processlist() {
 }
 
 set_trg_func() {
-   if [ -f "$OPT_FUNCTION" ]; then
-      source $OPT_FUNCTION
+   local func="$1"
+   if [ -f "$func" ]; then
+      # Trigger function is a file with Bash code; source it.
+      . "$func"
       TRIGGER_FUNCTION="trg_plugin"
+      return 0  # success
    else
-      TRIGGER_FUNCTION="trg_$OPT_FUNCTION"
+      # Trigger function is name of a built-in function.
+      func=$(echo "$func" | tr [:upper:] [:lower:])
+      if [ "$func" = "status" -o "$func" = "processlist" ]; then
+         TRIGGER_FUNCTION="trg_$func"
+         return 0  # success
+      fi
    fi
+   return 1  # error
 }
 
 trg_status() {
@@ -1119,9 +1136,6 @@ main() {
    # Make a secure tmpdir.
    mk_tmpdir
 
-   # Set TRIGGER_FUNCTION based on --function.
-   set_trg_func
-
    # Stalk while oktorun.
    stalk
 
@@ -1147,10 +1161,17 @@ if    [ "${0##*/}" = "$TOOL" ] \
    [ -n "$(mysqladmin --help)" ] \
       || die "Cannot execute mysqladmin.  Check that it is in PATH."
 
+
    # Parse command line options.  We must do this first so we can
    # see if --daemonize was specified.
    mk_tmpdir
    parse_options "$0" "$@"
+
+   # Verify and set TRIGGER_FUNCTION based on --function.
+   if ! set_trg_func "$OPT_FUNCTION"; then
+      option_error "Invalid --function value: $OPT_FUNCTION"
+   fi
+
    usage_or_errors "$0"
    po_status=$?
    rm_tmpdir
diff --git a/lib/bash/parse_options.sh b/lib/bash/parse_options.sh
index 3c2ac8c9..59aff04a 100644
--- a/lib/bash/parse_options.sh
+++ b/lib/bash/parse_options.sh
@@ -65,9 +65,6 @@ PO_DIR="$TMPDIR/po"  # Directory with program option spec files
 # Required Global Variables:
 #   TIMDIR  - Temp directory set by <set_TMPDIR()>.
 #   TOOL    - Tool's name.
-#
-# Optional Global Variables:
-#   OPT_ERR - Command line option error message.
 usage() {
    local file="$1"
 
@@ -140,6 +137,12 @@ usage_or_errors() {
    return 0
 }
 
+option_error() {
+   local err="$1"
+   OPT_ERRS=$(($OPT_ERRS + 1))
+   echo "$err" >&2
+}
+
 # Sub: parse_options
 #   Parse Perl POD options from a program file.
 #
@@ -384,8 +387,7 @@ _parse_command_line() {
       if [ "$next_opt_is_val" ]; then
          next_opt_is_val=""
          if [ $# -eq 0 ] || [ $(expr "$opt" : "-") -eq 1 ]; then
-            OPT_ERRS=$(($OPT_ERRS + 1))
-            echo "$real_opt requires a $required_arg argument" >&2
+            option_error "$real_opt requires a $required_arg argument"
             continue
          fi
          val="$opt"
@@ -425,8 +427,7 @@ _parse_command_line() {
          else
             spec=$(grep "^short form:-$opt\$" "$TMPDIR"/po/* | cut -d ':' -f 1)
             if [ -z "$spec"  ]; then
-               OPT_ERRS=$(($OPT_ERRS + 1))
-               echo "Unknown option: $real_opt" >&2
+               option_error "Unknown option: $real_opt"
                continue
             fi
          fi
@@ -446,8 +447,7 @@ _parse_command_line() {
          else
             # Option does not take a value.
             if [ "$val" ]; then
-               OPT_ERRS=$(($OPT_ERRS + 1))
-               echo "Option $real_opt does not take a value" >&2
+               option_error "Option $real_opt does not take a value"
                continue
             fi 
             if [ "$opt_is_negated" ]; then