From a2f8cc7f9080d4058a87719bc67ae762ec8c1c46 Mon Sep 17 00:00:00 2001
From: chrfranke <chrfranke@4ea69e1a-61f1-4043-bf83-b5c94c648137>
Date: Tue, 7 Jul 2009 19:28:29 +0000
Subject: [PATCH] Replace global 'con->...' variables used for selective
 self-tests by local variables.

git-svn-id: https://smartmontools.svn.sourceforge.net/svnroot/smartmontools/trunk@2823 4ea69e1a-61f1-4043-bf83-b5c94c648137
---
 sm5/CHANGELOG    |  5 ++++-
 sm5/atacmds.cpp  | 46 +++++++++++++++++++++++++---------------------
 sm5/atacmds.h    | 43 ++++++++++++++++++++++++++++++++++++++++---
 sm5/ataprint.cpp |  5 +++--
 sm5/ataprint.h   |  3 ++-
 sm5/extern.h     | 17 +----------------
 sm5/smartctl.cpp | 27 ++++++++++++++-------------
 sm5/smartd.cpp   | 16 ++++++++--------
 sm5/utility.h    |  7 +------
 9 files changed, 98 insertions(+), 71 deletions(-)

diff --git a/sm5/CHANGELOG b/sm5/CHANGELOG
index 2c4b15f2b..93bf6365b 100644
--- a/sm5/CHANGELOG
+++ b/sm5/CHANGELOG
@@ -1,6 +1,6 @@
 CHANGELOG for smartmontools
 
-$Id: CHANGELOG,v 1.818 2009/07/06 02:46:47 geoffk1 Exp $
+$Id: CHANGELOG,v 1.819 2009/07/07 19:28:29 chrfranke Exp $
 
 The most recent version of this file is:
 http://smartmontools.cvs.sourceforge.net/smartmontools/sm5/CHANGELOG?view=markup
@@ -41,6 +41,9 @@ NOTES FOR FUTURE RELEASES: see TODO file.
 
 <DEVELOPERS: ADDITIONS TO THE CHANGE LOG GO JUST BELOW HERE, PLEASE>
 
+  [CF] Replace global 'con->...' variables used for selective self-tests
+       by local variables.
+
   [GK] Add names for some attributes used in Samsung MLC drives:
        178-180 & 183
 
diff --git a/sm5/atacmds.cpp b/sm5/atacmds.cpp
index 500ab8e0c..e1c7f427d 100644
--- a/sm5/atacmds.cpp
+++ b/sm5/atacmds.cpp
@@ -40,7 +40,7 @@
 
 #include <algorithm> // std::sort
 
-const char *atacmds_c_cvsid="$Id: atacmds.cpp,v 1.218 2009/07/06 02:46:47 geoffk1 Exp $"
+const char *atacmds_c_cvsid="$Id: atacmds.cpp,v 1.219 2009/07/07 19:28:29 chrfranke Exp $"
 ATACMDS_H_CVSID CONFIG_H_CVSID EXTERN_H_CVSID INT64_H_CVSID SCSIATA_H_CVSID UTILITY_H_CVSID;
 
 // for passing global control variables
@@ -1086,7 +1086,8 @@ int ataReadSelectiveSelfTestLog(ata_device * device, struct ata_selective_self_t
 }
 
 // Writes the selective self-test log (log #9)
-int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * sv, uint64_t num_sectors)
+int ataWriteSelectiveSelfTestLog(ata_device * device, ata_selective_selftest_args & args,
+                                 const ata_smart_values * sv, uint64_t num_sectors)
 {
   // Disk size must be known
   if (!num_sectors) {
@@ -1114,10 +1115,10 @@ int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * s
 
   // Set start/end values based on old spans for special -t select,... options
   int i;
-  for (i=0; i<con->smartselectivenumspans; i++) {
-    char mode = con->smartselectivemode[i];
-    uint64_t start = con->smartselectivespan[i][0];
-    uint64_t end   = con->smartselectivespan[i][1];
+  for (i = 0; i < args.num_spans; i++) {
+    int mode = args.span[i].mode;
+    uint64_t start = args.span[i].start;
+    uint64_t end   = args.span[i].end;
     if (mode == SEL_CONT) {// redo or next dependig on last test status
       switch (sv->self_test_exec_status >> 4) {
         case 1: case 2: // Aborted/Interrupted by host
@@ -1183,9 +1184,9 @@ int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * s
       return -1;
     }
     // Return the actual mode and range to caller.
-    con->smartselectivemode[i] = mode;
-    con->smartselectivespan[i][0] = start;
-    con->smartselectivespan[i][1] = end;
+    args.span[i].mode  = mode;
+    args.span[i].start = start;
+    args.span[i].end   = end;
   }
 
   // Clear spans
@@ -1193,9 +1194,9 @@ int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * s
     memset(data->span+i, 0, sizeof(struct test_span));
   
   // Set spans for testing 
-  for (i=0; i<con->smartselectivenumspans; i++){
-    data->span[i].start = con->smartselectivespan[i][0];
-    data->span[i].end   = con->smartselectivespan[i][1];
+  for (i = 0; i < args.num_spans; i++){
+    data->span[i].start = args.span[i].start;
+    data->span[i].end   = args.span[i].end;
   }
 
   // host must initialize to zero before initiating selective self-test
@@ -1203,10 +1204,10 @@ int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * s
   data->currentspan=0;
   
   // Perform off-line scan after selective test?
-  if (1 == con->scanafterselect)
+  if (args.scan_after_select == 1)
     // NO
     data->flags &= ~SELECTIVE_FLAG_DOSCAN;
-  else if (2 == con->scanafterselect)
+  else if (args.scan_after_select == 2)
     // YES
     data->flags |= SELECTIVE_FLAG_DOSCAN;
   
@@ -1215,8 +1216,8 @@ int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * s
   data->flags &= ~(SELECTIVE_FLAG_PENDING);
 
   // modify pending time?
-  if (con->pendingtime)
-    data->pendingtime=(unsigned short)(con->pendingtime-1);
+  if (args.pending_time)
+    data->pendingtime = (unsigned short)(args.pending_time-1);
 
   // Set checksum to zero, then compute checksum
   data->checksum=0;
@@ -1451,7 +1452,9 @@ int ataSmartStatus2(ata_device * device){
 
 // This is the way to execute ALL tests: offline, short self-test,
 // extended self test, with and without captive mode, etc.
-int ataSmartTest(ata_device * device, int testtype, const ata_smart_values * sv, uint64_t num_sectors)
+// TODO: Move to ataprint.cpp ?
+int ataSmartTest(ata_device * device, int testtype, const ata_selective_selftest_args & selargs,
+                 const ata_smart_values * sv, uint64_t num_sectors)
 {
   char cmdmsg[128]; const char *type, *captive;
   int errornum, cap, retval, select=0;
@@ -1480,7 +1483,8 @@ int ataSmartTest(ata_device * device, int testtype, const ata_smart_values * sv,
   
   // If doing a selective self-test, first use WRITE_LOG to write the
   // selective self-test log.
-  if (select && (retval=ataWriteSelectiveSelfTestLog(device, sv, num_sectors))) {
+  ata_selective_selftest_args selargs_io = selargs; // filled with info about actual spans
+  if (select && (retval = ataWriteSelectiveSelfTestLog(device, selargs_io, sv, num_sectors))) {
     if (retval==-4)
       pout("Can't start selective self-test without aborting current test: use '-X' option to smartctl.\n");
     return retval;
@@ -1496,10 +1500,10 @@ int ataSmartTest(ata_device * device, int testtype, const ata_smart_values * sv,
   if (select) {
     int i;
     pout("SPAN         STARTING_LBA           ENDING_LBA\n");
-    for (i = 0; i < con->smartselectivenumspans; i++)
+    for (i = 0; i < selargs_io.num_spans; i++)
       pout("   %d %20"PRId64" %20"PRId64"\n", i,
-           con->smartselectivespan[i][0],
-           con->smartselectivespan[i][1]);
+           selargs_io.span[i].start,
+           selargs_io.span[i].end);
   }
   
   // Now send the command to test
diff --git a/sm5/atacmds.h b/sm5/atacmds.h
index 4b3d1face..5d61f947a 100644
--- a/sm5/atacmds.h
+++ b/sm5/atacmds.h
@@ -26,7 +26,7 @@
 #ifndef ATACMDS_H_
 #define ATACMDS_H_
 
-#define ATACMDS_H_CVSID "$Id: atacmds.h,v 1.107 2009/04/16 21:24:08 chrfranke Exp $\n"
+#define ATACMDS_H_CVSID "$Id: atacmds.h,v 1.108 2009/07/07 19:28:29 chrfranke Exp $\n"
 
 #include "dev_interface.h" // ata_device
 
@@ -591,6 +591,41 @@ struct ata_sct_temperature_history_table
 #pragma pack()
 ASSERT_SIZEOF_STRUCT(ata_sct_temperature_history_table, 512);
 
+// Possible values for span_args.mode
+enum {
+  SEL_RANGE, // MIN-MAX
+  SEL_REDO,  // redo this
+  SEL_NEXT,  // do next range
+  SEL_CONT   // redo or next depending of last test status
+};
+
+// Arguments for selective self-test
+struct ata_selective_selftest_args
+{
+  // Arguments for each span
+  struct span_args
+  {
+    uint64_t start;   // First block
+    uint64_t end;     // Last block
+    int mode;         // SEL_*, see above
+
+    span_args()
+      : start(0), end(0), mode(SEL_RANGE) { }
+  };
+
+  span_args span[5];  // Range and mode for 5 spans
+  int num_spans;      // Number of spans
+  int pending_time;   // One plus time in minutes to wait after powerup before restarting
+                      // interrupted offline scan after selective self-test.
+  int scan_after_select; // Run offline scan after selective self-test:
+                      // 0: don't change,
+                      // 1: turn off scan after selective self-test,
+                      // 2: turn on scan after selective self-test.
+
+  ata_selective_selftest_args()
+    : num_spans(0), pending_time(0), scan_after_select(0) { }
+};
+
 
 // Get information from drive
 int ataReadHDIdentity(ata_device * device, struct ata_identify_device *buf);
@@ -646,7 +681,8 @@ int ataSmartShortSelfTest (ata_device * device);
 int ataSmartShortCapSelfTest (ata_device * device);
 int ataSmartExtendCapSelfTest (ata_device * device);
 int ataSmartSelfTestAbort (ata_device * device);
-int ataWriteSelectiveSelfTestLog(ata_device * device, const ata_smart_values * sv, uint64_t num_sectors);
+int ataWriteSelectiveSelfTestLog(ata_device * device, ata_selective_selftest_args & args,
+                                 const ata_smart_values * sv, uint64_t num_sectors);
 
 // Returns the latest compatibility of ATA/ATAPI Version the device
 // supports. Returns -1 if Version command is not supported
@@ -700,7 +736,8 @@ inline bool isSCTFeatureControlCapable(const ata_identify_device *drive)
 inline bool isSCTDataTableCapable(const ata_identify_device *drive)
   { return ((drive->words088_255[206-88] & 0x21) == 0x21); } // 0x20 = SCT Data Table support
 
-int ataSmartTest(ata_device * device, int testtype, const ata_smart_values *data, uint64_t num_sectors);
+int ataSmartTest(ata_device * device, int testtype, const ata_selective_selftest_args & args,
+                 const ata_smart_values * sv, uint64_t num_sectors);
 
 int TestTime(const ata_smart_values * data, int testtype);
 
diff --git a/sm5/ataprint.cpp b/sm5/ataprint.cpp
index 12363f375..3d315213b 100644
--- a/sm5/ataprint.cpp
+++ b/sm5/ataprint.cpp
@@ -44,7 +44,7 @@
 #include "utility.h"
 #include "knowndrives.h"
 
-const char *ataprint_c_cvsid="$Id: ataprint.cpp,v 1.213 2009/07/05 17:16:38 chrfranke Exp $"
+const char *ataprint_c_cvsid="$Id: ataprint.cpp,v 1.214 2009/07/07 19:28:29 chrfranke Exp $"
 ATACMDNAMES_H_CVSID ATACMDS_H_CVSID ATAPRINT_H_CVSID CONFIG_H_CVSID EXTERN_H_CVSID INT64_H_CVSID KNOWNDRIVES_H_CVSID SMARTCTL_H_CVSID UTILITY_H_CVSID;
 
 // for passing global control variables
@@ -2374,7 +2374,8 @@ int ataPrintMain (ata_device * device, const ata_print_options & options)
 
   // Now do the test.  Note ataSmartTest prints its own error/success
   // messages
-  if (ataSmartTest(device, options.smart_selftest_type, &smartval, get_num_sectors(&drive)))
+  if (ataSmartTest(device, options.smart_selftest_type, options.smart_selective_args,
+                   &smartval, get_num_sectors(&drive)                                ))
     failuretest(OPTIONAL_CMD, returnval|=FAILSMART);
   else {  
     // Tell user how long test will take to complete.  This is tricky
diff --git a/sm5/ataprint.h b/sm5/ataprint.h
index 404cb398f..a49389217 100644
--- a/sm5/ataprint.h
+++ b/sm5/ataprint.h
@@ -26,7 +26,7 @@
 #ifndef ATAPRINT_H_
 #define ATAPRINT_H_
 
-#define ATAPRINT_H_CVSID "$Id: ataprint.h,v 1.42 2009/07/05 17:16:38 chrfranke Exp $\n"
+#define ATAPRINT_H_CVSID "$Id: ataprint.h,v 1.43 2009/07/07 19:28:29 chrfranke Exp $\n"
 
 #include <vector>
 
@@ -70,6 +70,7 @@ struct ata_print_options
   bool smart_auto_save_disable, smart_auto_save_enable;
 
   int smart_selftest_type; // OFFLINE_FULL_SCAN, ..., see atacmds.h. -1 for no test
+  ata_selective_selftest_args smart_selective_args; // Extra args for selective self-test
 
   unsigned sct_temp_int;
   bool sct_temp_int_pers;
diff --git a/sm5/extern.h b/sm5/extern.h
index fc841f6e2..a1b067e79 100644
--- a/sm5/extern.h
+++ b/sm5/extern.h
@@ -25,27 +25,12 @@
 #ifndef EXTERN_H_
 #define EXTERN_H_
 
-#define EXTERN_H_CVSID "$Id: extern.h,v 1.62 2009/06/20 19:11:04 chrfranke Exp $\n"
+#define EXTERN_H_CVSID "$Id: extern.h,v 1.63 2009/07/07 19:28:29 chrfranke Exp $\n"
 
 // Block used for global control/communications.  If you need more
 // global variables, this should be the only place that you need to
 // add them.
 typedef struct smartmonctrl_s {
-  // TODO: Use local struct for selective self test parameters
-  // spans for selective self-test
-  uint64_t smartselectivespan[5][2];
-  // mode for each span, see SEL_* in utility.h
-  char smartselectivemode[5];
-  // number of spans
-  int smartselectivenumspans;
-  // one plus time in minutes to wait after powerup before restarting
-  // interrupted offline scan after selective self-test.
-  int  pendingtime;
-  // run offline scan after selective self-test.  0: don't change, 1:
-  // turn off scan after selective self-test, 2: turn on scan after
-  // selective self-test.
-  unsigned char scanafterselect;
-
   bool printing_switchable;
   bool dont_print;
   bool dont_print_serial;
diff --git a/sm5/smartctl.cpp b/sm5/smartctl.cpp
index fced9d9a3..2bc5ff5f5 100644
--- a/sm5/smartctl.cpp
+++ b/sm5/smartctl.cpp
@@ -63,7 +63,7 @@ extern const char *os_solaris_ata_s_cvsid;
 extern const char *cciss_c_cvsid;
 #endif
 extern const char *atacmdnames_c_cvsid, *atacmds_c_cvsid, *ataprint_c_cvsid, *knowndrives_c_cvsid, *os_XXXX_c_cvsid, *scsicmds_c_cvsid, *scsiprint_c_cvsid, *utility_c_cvsid;
-const char* smartctl_c_cvsid="$Id: smartctl.cpp,v 1.203 2009/07/05 18:01:58 chrfranke Exp $"
+const char* smartctl_c_cvsid="$Id: smartctl.cpp,v 1.204 2009/07/07 19:28:29 chrfranke Exp $"
 ATACMDS_H_CVSID ATAPRINT_H_CVSID CONFIG_H_CVSID EXTERN_H_CVSID INT64_H_CVSID KNOWNDRIVES_H_CVSID SCSICMDS_H_CVSID SCSIPRINT_H_CVSID SMARTCTL_H_CVSID UTILITY_H_CVSID;
 
 // This is a block containing all the "control variables".  We declare
@@ -627,11 +627,11 @@ const char * parse_options(int argc, char** argv,
         testcnt++;
         ataopts.smart_selftest_type = CONVEYANCE_SELF_TEST;
       } else if (!strcmp(optarg,"afterselect,on")) {
-	// scan remainder of disk after doing selected segments
-	con->scanafterselect=2;
+        // scan remainder of disk after doing selected segment
+        ataopts.smart_selective_args.scan_after_select = 2;
       } else if (!strcmp(optarg,"afterselect,off")) {
-	// don't scan remainder of disk after doing selected segments
-	con->scanafterselect=1;
+        // don't scan remainder of disk after doing selected segments
+        ataopts.smart_selective_args.scan_after_select = 1;
       } else if (!strncmp(optarg,"pending,",strlen("pending,"))) {
 	// parse number of minutes that test should be pending
 	int i;
@@ -645,7 +645,7 @@ const char * parse_options(int argc, char** argv,
 	  sprintf(extraerror, "Option -t pending,N (N=%d) must have 0 <= N <= 65535\n", i);
           badarg = true;
 	} else {
-	  con->pendingtime=i+1;
+          ataopts.smart_selective_args.pending_time = i+1;
 	}
       } else if (!strncmp(optarg,"select",strlen("select"))) {
         testcnt++;
@@ -655,7 +655,7 @@ const char * parse_options(int argc, char** argv,
 	  sprintf(extraerror, "Option -t select,M-N must have non-negative integer M and N\n");
           badarg = true;
         } else {
-          if (con->smartselectivenumspans >= 5 || start > stop) {
+          if (ataopts.smart_selective_args.num_spans >= 5 || start > stop) {
             if (start > stop) {
               sprintf(extraerror, "ERROR: Start LBA (%"PRIu64") > ending LBA (%"PRId64") in argument \"%s\"\n",
                 start, stop, optarg);
@@ -665,10 +665,10 @@ const char * parse_options(int argc, char** argv,
             }
             badarg = true;
           }
-          con->smartselectivespan[con->smartselectivenumspans][0] = start;
-          con->smartselectivespan[con->smartselectivenumspans][1] = stop;
-          con->smartselectivemode[con->smartselectivenumspans] = mode;
-          con->smartselectivenumspans++;
+          ataopts.smart_selective_args.span[ataopts.smart_selective_args.num_spans].start = start;
+          ataopts.smart_selective_args.span[ataopts.smart_selective_args.num_spans].end   = stop;
+          ataopts.smart_selective_args.span[ataopts.smart_selective_args.num_spans].mode  = mode;
+          ataopts.smart_selective_args.num_spans++;
           ataopts.smart_selftest_type = SELECTIVE_SELF_TEST;
         }
       } else if (!strncmp(optarg, "scttempint,", sizeof("scstempint,")-1)) {
@@ -790,10 +790,11 @@ const char * parse_options(int argc, char** argv,
 
   // error message if user has set selective self-test options without
   // asking for a selective self-test
-  if ((con->pendingtime || con->scanafterselect) && !con->smartselectivenumspans){
+  if (   (ataopts.smart_selective_args.pending_time || ataopts.smart_selective_args.scan_after_select)
+      && !ataopts.smart_selective_args.num_spans) {
     con->dont_print = false;
     printslogan();
-    if (con->pendingtime)
+    if (ataopts.smart_selective_args.pending_time)
       pout("\nERROR: smartctl -t pending,N must be used with -t select,N-M.\n");
     else
       pout("\nERROR: smartctl -t afterselect,(on|off) must be used with -t select,N-M.\n");
diff --git a/sm5/smartd.cpp b/sm5/smartd.cpp
index 705748e3e..ee45607e6 100644
--- a/sm5/smartd.cpp
+++ b/sm5/smartd.cpp
@@ -136,7 +136,7 @@ extern const char *os_solaris_ata_s_cvsid;
 #ifdef _WIN32
 extern const char *daemon_win32_c_cvsid, *hostname_win32_c_cvsid, *syslog_win32_c_cvsid;
 #endif
-const char *smartd_c_cvsid="$Id: smartd.cpp,v 1.448 2009/06/27 16:58:29 chrfranke Exp $"
+const char *smartd_c_cvsid="$Id: smartd.cpp,v 1.449 2009/07/07 19:28:29 chrfranke Exp $"
 ATACMDS_H_CVSID CONFIG_H_CVSID
 #ifdef DAEMON_WIN32_H_CVSID
 DAEMON_WIN32_H_CVSID
@@ -2368,7 +2368,7 @@ static int DoATASelfTest(const dev_config & cfg, dev_state & state, ata_device *
   }
   
   // Check for capability to do the test
-  int dotest = -1; char mode = 0;
+  int dotest = -1, mode = 0;
   const char *testname = 0;
   switch (testtype) {
   case 'O':
@@ -2435,16 +2435,16 @@ static int DoATASelfTest(const dev_config & cfg, dev_state & state, ata_device *
 
   if (dotest == SELECTIVE_SELF_TEST) {
     // Set test span
-    con->smartselectivenumspans = 1; // TODO: Use parameters instead of globals
-    con->smartselectivemode[0] = mode;
-    con->smartselectivespan[0][0] = con->smartselectivespan[0][1] = 0;
-    if (ataWriteSelectiveSelfTestLog(device, &data, state.num_sectors)) {
+    ata_selective_selftest_args selargs;
+    selargs.num_spans = 1;
+    selargs.span[0].mode = mode;
+    if (ataWriteSelectiveSelfTestLog(device, selargs, &data, state.num_sectors)) {
       PrintOut(LOG_CRIT, "Device: %s, prepare %sTest failed\n", name, testname);
       return 1;
     }
-    uint64_t start = con->smartselectivespan[0][0], end = con->smartselectivespan[0][1];
+    uint64_t start = selargs.span[0].start, end = selargs.span[0].end;
     PrintOut(LOG_INFO, "Device: %s, %s test span at LBA %"PRIu64" - %"PRIu64" (%"PRIu64" sectors, %u%% - %u%% of disk).\n",
-      name, (con->smartselectivemode[0] == SEL_NEXT ? "next" : "redo"),
+      name, (selargs.span[0].mode == SEL_NEXT ? "next" : "redo"),
       start, end, end - start + 1,
       (unsigned)((100 * start + state.num_sectors/2) / state.num_sectors),
       (unsigned)((100 * end   + state.num_sectors/2) / state.num_sectors));
diff --git a/sm5/utility.h b/sm5/utility.h
index 6f74e03b5..892e42c82 100644
--- a/sm5/utility.h
+++ b/sm5/utility.h
@@ -25,7 +25,7 @@
 #ifndef UTILITY_H_
 #define UTILITY_H_
 
-#define UTILITY_H_CVSID "$Id: utility.h,v 1.67 2009/06/20 19:11:05 chrfranke Exp $\n"
+#define UTILITY_H_CVSID "$Id: utility.h,v 1.68 2009/07/07 19:28:29 chrfranke Exp $\n"
 
 #include <time.h>
 #include <sys/types.h> // for regex.h (according to POSIX)
@@ -88,11 +88,6 @@ int split_report_arg(char *s, int *i);
 // Function for processing -c option in smartctl and smartd
 int split_report_arg2(char *s, int *i);
 
-// Possible values for smartselectivemode
-#define SEL_RANGE            0 // MIN-MAX
-#define SEL_REDO             1 // redo this
-#define SEL_NEXT             2 // do next range
-#define SEL_CONT             3 // redo or next depending of last test status
 // Function for processing -t selective... option in smartctl
 int split_selective_arg(char *s, uint64_t *start, uint64_t *stop, int *mode);
 
-- 
GitLab