From a19411233cb93feb676544c59219a619f5fc6358 Mon Sep 17 00:00:00 2001
From: ballen4705 <ballen4705@4ea69e1a-61f1-4043-bf83-b5c94c648137>
Date: Thu, 24 Oct 2002 15:25:27 +0000
Subject: [PATCH] Check that device has ALL smart capabilities needed before
 adding to list of devices.  Close ATA file descriptors rather than leaving
 them open.

git-svn-id: https://smartmontools.svn.sourceforge.net/svnroot/smartmontools/trunk@115 4ea69e1a-61f1-4043-bf83-b5c94c648137
---
 sm5/TODO       |  9 ++++++++-
 sm5/smartd.c   | 34 +++++++++++++++++++++-------------
 sm5/smartd.cpp | 34 +++++++++++++++++++++-------------
 3 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/sm5/TODO b/sm5/TODO
index 38ec86547..cd8574a90 100644
--- a/sm5/TODO
+++ b/sm5/TODO
@@ -4,7 +4,7 @@ Home page of code is: http://smartmontools.sourceforge.net
 
 Copyright (C) 2002 Bruce Allen <smartmontools-support@lists.sourceforge.net>
 
-$Id: TODO,v 1.13 2002/10/24 11:38:11 ballen4705 Exp $
+$Id: TODO,v 1.14 2002/10/24 15:25:27 ballen4705 Exp $
 
 This program is free software; you can redistribute it and/or modify it
 under the terms of the GNU General Public License as published by the Free
@@ -37,6 +37,13 @@ Modifications
 Change smartd so that it also monitors usage as well as prefail attributes for failure
 or changes.  Make this a command line option
 
+Perhaps change smartd to look in /proc/ide and /proc/scsi to see what exists? If something
+doesn't exit then don't try to open it?
+
+Currently smartd looks at attribute values and thresholds, then prints out if there is an
+error condition.  Make it also look at the smart status, if that is enabled and working and
+do that test as well.  Perhaps also see if the number of device errors has changed.
+
 Change smartd so that it monitors the ATA disk error log, and if the number of errors changes,
 log an entry.
 
diff --git a/sm5/smartd.c b/sm5/smartd.c
index bcf3ca2be..4266f28ab 100644
--- a/sm5/smartd.c
+++ b/sm5/smartd.c
@@ -37,7 +37,7 @@
 #include "ataprint.h"
 
 extern const char *CVSid1, *CVSid2;
-const char *CVSid3="$Id: smartd.c,v 1.26 2002/10/24 15:03:15 ballen4705 Exp $" 
+const char *CVSid3="$Id: smartd.c,v 1.27 2002/10/24 15:25:27 ballen4705 Exp $" 
 CVSID1 CVSID4 CVSID7;
 
 int daemon_init(void){
@@ -137,21 +137,27 @@ void atadevicescan ( atadevices_t *devices){
       printout(LOG_INFO,"Device: %s, Found but not SMART capable, or couldn't enable SMART\n",device);
       continue;
     }
-     
-    // device exists, and does SMART.  Add to list
-    devices[numatadevices].fd = fd;
-    strcpy(devices[numatadevices].devicename, device);
-    devices[numatadevices].drive = drive;
+
+    // Does device support read values and read thresholds?  We should
+    // modify this next block for devices that do support SMART status
+    // but don't support read values and read thresholds.
     if (ataReadSmartValues (fd,&devices[numatadevices].smartval)){
+      close(fd);
       printout(LOG_INFO,"Device: %s, Read SMART Values Failed\n",device);
+      continue;
     }
-    
-    if (ataReadSmartThresholds (fd,&devices[numatadevices].smartthres)){
+    else if (ataReadSmartThresholds (fd,&devices[numatadevices].smartthres)){
+      close(fd);
       printout(LOG_INFO,"Device: %s, Read SMART Thresholds Failed\n",device);
+      continue;
     }
-    
+
+    // device exists, and does SMART.  Add to list
     printout(LOG_INFO,"%s Found and is SMART capable\n",device);
-    
+    devices[numatadevices].fd = fd;
+    strcpy(devices[numatadevices].devicename, device);
+    devices[numatadevices].drive = drive;
+        
     // This makes NO sense.  We may want to know if the drive supports
     // Offline Surface Scan, for example.  But checking if it supports
     // self-tests seems useless. In any case, smartd NEVER uses this
@@ -163,9 +169,11 @@ void atadevicescan ( atadevices_t *devices){
   }
 }
 
-// This function is hard to read and ought to be rewritten
-// A couple of obvious questions -- why isn't fd always closed if not used?
-// Why in the world is the four-byte integer cast to a pointer to an eight-byte object??
+
+
+// This function is hard to read and ought to be rewritten. Why in the
+// world is the four-byte integer cast to a pointer to an eight-byte
+// object??
 void scsidevicescan ( scsidevices_t *devices){
   int i, fd, smartsupport;
   unsigned char  tBuf[4096];
diff --git a/sm5/smartd.cpp b/sm5/smartd.cpp
index de8484437..7640d330d 100644
--- a/sm5/smartd.cpp
+++ b/sm5/smartd.cpp
@@ -37,7 +37,7 @@
 #include "ataprint.h"
 
 extern const char *CVSid1, *CVSid2;
-const char *CVSid3="$Id: smartd.cpp,v 1.26 2002/10/24 15:03:15 ballen4705 Exp $" 
+const char *CVSid3="$Id: smartd.cpp,v 1.27 2002/10/24 15:25:27 ballen4705 Exp $" 
 CVSID1 CVSID4 CVSID7;
 
 int daemon_init(void){
@@ -137,21 +137,27 @@ void atadevicescan ( atadevices_t *devices){
       printout(LOG_INFO,"Device: %s, Found but not SMART capable, or couldn't enable SMART\n",device);
       continue;
     }
-     
-    // device exists, and does SMART.  Add to list
-    devices[numatadevices].fd = fd;
-    strcpy(devices[numatadevices].devicename, device);
-    devices[numatadevices].drive = drive;
+
+    // Does device support read values and read thresholds?  We should
+    // modify this next block for devices that do support SMART status
+    // but don't support read values and read thresholds.
     if (ataReadSmartValues (fd,&devices[numatadevices].smartval)){
+      close(fd);
       printout(LOG_INFO,"Device: %s, Read SMART Values Failed\n",device);
+      continue;
     }
-    
-    if (ataReadSmartThresholds (fd,&devices[numatadevices].smartthres)){
+    else if (ataReadSmartThresholds (fd,&devices[numatadevices].smartthres)){
+      close(fd);
       printout(LOG_INFO,"Device: %s, Read SMART Thresholds Failed\n",device);
+      continue;
     }
-    
+
+    // device exists, and does SMART.  Add to list
     printout(LOG_INFO,"%s Found and is SMART capable\n",device);
-    
+    devices[numatadevices].fd = fd;
+    strcpy(devices[numatadevices].devicename, device);
+    devices[numatadevices].drive = drive;
+        
     // This makes NO sense.  We may want to know if the drive supports
     // Offline Surface Scan, for example.  But checking if it supports
     // self-tests seems useless. In any case, smartd NEVER uses this
@@ -163,9 +169,11 @@ void atadevicescan ( atadevices_t *devices){
   }
 }
 
-// This function is hard to read and ought to be rewritten
-// A couple of obvious questions -- why isn't fd always closed if not used?
-// Why in the world is the four-byte integer cast to a pointer to an eight-byte object??
+
+
+// This function is hard to read and ought to be rewritten. Why in the
+// world is the four-byte integer cast to a pointer to an eight-byte
+// object??
 void scsidevicescan ( scsidevices_t *devices){
   int i, fd, smartsupport;
   unsigned char  tBuf[4096];
-- 
GitLab