From 69b5037820c2c503c0f6ae6edb5a81bb2b392284 Mon Sep 17 00:00:00 2001 From: chrfranke <chrfranke@4ea69e1a-61f1-4043-bf83-b5c94c648137> Date: Mon, 26 Oct 2009 22:05:54 +0000 Subject: [PATCH] Add smart pointer class template to manage device object pointers. Remove related 'delete' calls and 'try/catch' blocks. git-svn-id: https://smartmontools.svn.sourceforge.net/svnroot/smartmontools/trunk@2971 4ea69e1a-61f1-4043-bf83-b5c94c648137 --- smartmontools/CHANGELOG | 3 + smartmontools/dev_interface.cpp | 23 ++--- smartmontools/dev_interface.h | 96 +++++++++++++++++++++ smartmontools/dev_legacy.cpp | 54 ++++++------ smartmontools/os_freebsd.cpp | 27 +++--- smartmontools/os_linux.cpp | 75 ++++++++--------- smartmontools/scsiata.cpp | 39 ++------- smartmontools/smartctl.cpp | 145 +++++++++++++++----------------- smartmontools/smartd.cpp | 39 ++++----- 9 files changed, 268 insertions(+), 233 deletions(-) diff --git a/smartmontools/CHANGELOG b/smartmontools/CHANGELOG index d8d135cd2..a3e694c15 100644 --- a/smartmontools/CHANGELOG +++ b/smartmontools/CHANGELOG @@ -43,6 +43,9 @@ NOTES FOR FUTURE RELEASES: see TODO file. <DEVELOPERS: ADDITIONS TO THE CHANGE LOG GO JUST BELOW HERE, PLEASE> + [CF] Add smart pointer class template to manage device object pointers. + Remove related 'delete' calls and 'try/catch' blocks. + [CF] do_release: Replace generation of '*.asc' by '*.md5' and '*.sha1'. [MS] attribute updates: diff --git a/smartmontools/dev_interface.cpp b/smartmontools/dev_interface.cpp index ae7fefe57..973705ff7 100644 --- a/smartmontools/dev_interface.cpp +++ b/smartmontools/dev_interface.cpp @@ -301,29 +301,22 @@ smart_device * smart_interface::get_smart_device(const char * name, const char * // Recurse to allocate base device, default is standard SCSI if (!*basetype) basetype = "scsi"; - dev = get_smart_device(name, basetype); - if (!dev) { + smart_device_auto_ptr basedev( get_smart_device(name, basetype) ); + if (!basedev) { set_err(EINVAL, "Type '%s+...': %s", sattype.c_str(), get_errmsg()); return 0; } // Result must be SCSI - if (!dev->is_scsi()) { - delete dev; + if (!basedev->is_scsi()) { set_err(EINVAL, "Type '%s+...': Device type '%s' is not SCSI", sattype.c_str(), basetype); return 0; } // Attach SAT tunnel - try { - ata_device * satdev = get_sat_device(sattype.c_str(), dev->to_scsi()); - if (!satdev) { - delete dev; - return 0; - } - return satdev; - } - catch (...) { - delete dev; throw; - } + ata_device * satdev = get_sat_device(sattype.c_str(), basedev->to_scsi()); + if (!satdev) + return 0; + basedev.release(); + return satdev; } else { diff --git a/smartmontools/dev_interface.h b/smartmontools/dev_interface.h index 5d932a8a8..eb53a01ef 100644 --- a/smartmontools/dev_interface.h +++ b/smartmontools/dev_interface.h @@ -21,6 +21,7 @@ #define DEV_INTERFACE_H_CVSID "$Id$\n" #include <stdarg.h> +#include <stdexcept> #include <string> #include <vector> @@ -514,6 +515,95 @@ inline void smart_device::this_is_scsi(scsi_device * scsi) } +///////////////////////////////////////////////////////////////////////////// +/// Smart pointer class for device pointers + +template <class Dev> +class any_device_auto_ptr +{ +public: + typedef Dev device_type; + + /// Construct from optional pointer to device + /// and optional pointer to base device. + explicit any_device_auto_ptr(device_type * dev = 0, + smart_device * base_dev = 0) + : m_dev(dev), m_base_dev(base_dev) { } + + /// Destructor deletes device object. + ~any_device_auto_ptr() throw() + { reset(); } + + /// Assign a new pointer. + /// Throws if a pointer is already assigned. + void operator=(device_type * dev) + { + if (m_dev) + fail(); + m_dev = dev; + } + + /// Delete device object and clear the pointer. + void reset() + { + if (m_dev) { + if (m_base_dev && m_dev->owns(m_base_dev)) + m_dev->release(m_base_dev); + delete m_dev; + } + m_dev = 0; + } + + /// Return the pointer and release ownership. + device_type * release() + { + device_type * dev = m_dev; + m_dev = 0; + return dev; + } + + /// Replace the pointer. + /// Used to call dev->autodetect_open(). + void replace(device_type * dev) + { m_dev = dev; } + + /// Return the pointer. + device_type * get() const + { return m_dev; } + + /// Pointer dereferencing. + device_type & operator*() const + { return *m_dev; } + + /// Pointer dereferencing. + device_type * operator->() const + { return m_dev; } + + /// For (ptr != 0) check. + operator bool() const + { return !!m_dev; } + + /// For (ptr == 0) check. + bool operator !() const + { return !m_dev; } + +private: + device_type * m_dev; + smart_device * m_base_dev; + + void fail() const + { throw std::logic_error("any_device_auto_ptr: wrong usage"); } + + // Prevent copy/assignment + any_device_auto_ptr(const any_device_auto_ptr<Dev> &); + void operator=(const any_device_auto_ptr<Dev> &); +}; + +typedef any_device_auto_ptr<smart_device> smart_device_auto_ptr; +typedef any_device_auto_ptr<ata_device> ata_device_auto_ptr; +typedef any_device_auto_ptr<scsi_device> scsi_device_auto_ptr; + + ///////////////////////////////////////////////////////////////////////////// // smart_device_list @@ -550,6 +640,12 @@ public: void push_back(smart_device * dev) { m_list.push_back(dev); } + void push_back(smart_device_auto_ptr & dev) + { + m_list.push_back(dev.get()); + dev.release(); + } + smart_device * at(unsigned i) { return m_list.at(i); } diff --git a/smartmontools/dev_legacy.cpp b/smartmontools/dev_legacy.cpp index 54c319d9e..ff430dbf6 100644 --- a/smartmontools/dev_legacy.cpp +++ b/smartmontools/dev_legacy.cpp @@ -3,7 +3,7 @@ * * Home page of code is: http://smartmontools.sourceforge.net * - * Copyright (C) 2008 Christian Franke <smartmontools-support@lists.sourceforge.net> + * Copyright (C) 2008-9 Christian Franke <smartmontools-support@lists.sourceforge.net> * * 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 @@ -405,44 +405,42 @@ smart_device * legacy_scsi_device::autodetect_open() int avail_len = req_buff[4] + 5; int len = (avail_len < req_len ? avail_len : req_len); if (len < 36) - return this; + return this; // Use INQUIRY to detect type - smart_device * newdev = 0; - try { - // 3ware ? - if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { - close(); + + // 3ware ? + if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { + close(); #if defined(_WIN32) || defined(__CYGWIN__) - set_err(EINVAL, "AMCC/3ware controller, please try changing device to %s,N", get_dev_name()); + set_err(EINVAL, "AMCC/3ware controller, please try changing device to %s,N", get_dev_name()); #else - set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" - "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); + set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" + "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); #endif - return this; - } + return this; + } - // Marvell ? - if (len >= 42 && !memcmp(req_buff + 36, "MVSATA", 6)) { // TODO: Linux-specific? - //pout("Device %s: using '-d marvell' for ATA disk with Marvell driver\n", get_dev_name()); - close(); - newdev = new legacy_marvell_device(smi(), get_dev_name(), get_req_type()); - newdev->open(); // TODO: Can possibly pass open fd - delete this; - return newdev; - } + // Marvell ? + if (len >= 42 && !memcmp(req_buff + 36, "MVSATA", 6)) { // TODO: Linux-specific? + //pout("Device %s: using '-d marvell' for ATA disk with Marvell driver\n", get_dev_name()); + close(); + smart_device_auto_ptr newdev( + new legacy_marvell_device(smi(), get_dev_name(), get_req_type()), + this + ); + newdev->open(); // TODO: Can possibly pass open fd + delete this; + return newdev.release(); + } - // SAT or USB ? - newdev = smi()->autodetect_sat_device(this, req_buff, len); + // SAT or USB ? + { + smart_device * newdev = smi()->autodetect_sat_device(this, req_buff, len); if (newdev) // NOTE: 'this' is now owned by '*newdev' return newdev; } - catch (...) { - // Cleanup if exception occurs after newdev was allocated - delete newdev; - throw; - } // Nothing special found return this; diff --git a/smartmontools/os_freebsd.cpp b/smartmontools/os_freebsd.cpp index 9f6ac3913..b80793b30 100644 --- a/smartmontools/os_freebsd.cpp +++ b/smartmontools/os_freebsd.cpp @@ -1218,27 +1218,22 @@ smart_device * freebsd_scsi_device::autodetect_open() return this; // Use INQUIRY to detect type - smart_device * newdev = 0; - try { - // 3ware ? - if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { - close(); - set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" - "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); - return this; - } - // SAT or USB ? - newdev = smi()->autodetect_sat_device(this, req_buff, len); + // 3ware ? + if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { + close(); + set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" + "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); + return this; + } + + // SAT or USB ? + { + smart_device * newdev = smi()->autodetect_sat_device(this, req_buff, len); if (newdev) // NOTE: 'this' is now owned by '*newdev' return newdev; } - catch (...) { - // Cleanup if exception occurs after newdev was allocated - delete newdev; - throw; - } // Nothing special found return this; diff --git a/smartmontools/os_linux.cpp b/smartmontools/os_linux.cpp index 821bf0f29..78d9c6984 100644 --- a/smartmontools/os_linux.cpp +++ b/smartmontools/os_linux.cpp @@ -930,19 +930,13 @@ smart_device * linux_megaraid_device::autodetect_open() printf("Got MegaRAID inquiry.. %s\n", req_buff+8); // Use INQUIRY to detect type - smart_device * newdev = 0; - try { + { // SAT or USB ? - newdev = smi()->autodetect_sat_device(this, req_buff, len); + ata_device * newdev = smi()->autodetect_sat_device(this, req_buff, len); if (newdev) // NOTE: 'this' is now owned by '*newdev' return newdev; } - catch (...) { - // Cleanup if exception occurs after newdev was allocated - delete newdev; - throw; - } // Nothing special found return this; @@ -2671,46 +2665,45 @@ smart_device * linux_scsi_device::autodetect_open() int avail_len = req_buff[4] + 5; int len = (avail_len < req_len ? avail_len : req_len); if (len < 36) - return this; + return this; // Use INQUIRY to detect type - smart_device * newdev = 0; - try { - // 3ware ? - if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { - close(); - set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" - "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); - return this; - } - // DELL? - if (!memcmp(req_buff + 8, "DELL PERC", 12) || !memcmp(req_buff + 8, "MegaRAID", 8)) { - close(); - set_err(EINVAL, "DELL or MegaRaid controller, please try adding '-d megaraid,N'"); - return this; - } - - // Marvell ? - if (len >= 42 && !memcmp(req_buff + 36, "MVSATA", 6)) { - //pout("Device %s: using '-d marvell' for ATA disk with Marvell driver\n", get_dev_name()); - close(); - newdev = new linux_marvell_device(smi(), get_dev_name(), get_req_type()); - newdev->open(); // TODO: Can possibly pass open fd - delete this; - return newdev; - } - // SAT or USB ? - newdev = smi()->autodetect_sat_device(this, req_buff, len); + // 3ware ? + if (!memcmp(req_buff + 8, "3ware", 5) || !memcmp(req_buff + 8, "AMCC", 4)) { + close(); + set_err(EINVAL, "AMCC/3ware controller, please try adding '-d 3ware,N',\n" + "you may need to replace %s with /dev/twaN or /dev/tweN", get_dev_name()); + return this; + } + + // DELL? + if (!memcmp(req_buff + 8, "DELL PERC", 12) || !memcmp(req_buff + 8, "MegaRAID", 8)) { + close(); + set_err(EINVAL, "DELL or MegaRaid controller, please try adding '-d megaraid,N'"); + return this; + } + + // Marvell ? + if (len >= 42 && !memcmp(req_buff + 36, "MVSATA", 6)) { + //pout("Device %s: using '-d marvell' for ATA disk with Marvell driver\n", get_dev_name()); + close(); + smart_device_auto_ptr newdev( + new linux_marvell_device(smi(), get_dev_name(), get_req_type()), + this + ); + newdev->open(); // TODO: Can possibly pass open fd + delete this; + return newdev.release(); + } + + // SAT or USB ? + { + smart_device * newdev = smi()->autodetect_sat_device(this, req_buff, len); if (newdev) // NOTE: 'this' is now owned by '*newdev' return newdev; } - catch (...) { - // Cleanup if exception occurs after newdev was allocated - delete newdev; - throw; - } // Nothing special found return this; diff --git a/smartmontools/scsiata.cpp b/smartmontools/scsiata.cpp index 236ed62cd..5c790107d 100644 --- a/smartmontools/scsiata.cpp +++ b/smartmontools/scsiata.cpp @@ -1307,40 +1307,11 @@ ata_device * smart_interface::autodetect_sat_device(scsi_device * scsidev, if (!scsidev->is_open()) return 0; - ata_device * atadev = 0; - try { - // SAT ? - if (inqdata && inqsize >= 36 && !memcmp(inqdata + 8, "ATA ", 8)) { // TODO: Linux-specific? - atadev = new sat_device(this, scsidev, ""); - if (has_sat_pass_through(atadev)) - return atadev; // Detected SAT - atadev->release(scsidev); - delete atadev; - } - -/* The new usbcypress_device(this, scsidev, "", 0x24) sends vendor specific comand to non-cypress devices. - * It's dangerous as other device may interpret such command as own valid vendor specific command. - * I commented it out untill problem resolved - */ -#if 0 - // USB ? - { - atadev = new usbcypress_device(this, scsidev, "", 0x24); - if (has_usbcypress_pass_through(atadev, - (inqdata && inqsize >= 36 ? (const char*)inqdata + 8 : 0), - (inqdata && inqsize >= 36 ? (const char*)inqdata + 16 : 0) )) - return atadev; // Detected USB - atadev->release(scsidev); - delete atadev; - } -#endif - } - catch (...) { - if (atadev) { - atadev->release(scsidev); - delete atadev; - } - throw; + // SAT ? + if (inqdata && inqsize >= 36 && !memcmp(inqdata + 8, "ATA ", 8)) { // TODO: Linux-specific? + ata_device_auto_ptr atadev( new sat_device(this, scsidev, "") , scsidev); + if (has_sat_pass_through(atadev.get())) + return atadev.release(); // Detected SAT } return 0; diff --git a/smartmontools/smartctl.cpp b/smartmontools/smartctl.cpp index e0f48c6fa..5e9f89f6b 100644 --- a/smartmontools/smartctl.cpp +++ b/smartmontools/smartctl.cpp @@ -877,92 +877,83 @@ int main_worker(int argc, char **argv) if (!smi()) return 1; - int retval = 0; - - smart_device * dev = 0; - try { - // define control block for external functions - smartmonctrl control; - con=&control; - - // Parse input arguments - ata_print_options ataopts; - scsi_print_options scsiopts; - const char * type = parse_options(argc, argv, ataopts, scsiopts); - - // '-d test' -> Report result of autodetection - bool print_type_only = (type && !strcmp(type, "test")); - if (print_type_only) - type = 0; - - const char * name = argv[argc-1]; - - if (!strcmp(name,"-")) { - // Parse "smartctl -r ataioctl,2 ..." output from stdin - if (type || print_type_only) { - pout("Smartctl: -d option is not allowed in conjunction with device name \"-\".\n"); - UsageSummary(); - return FAILCMD; - } - dev = get_parsed_ata_device(smi(), name); - } - else - // get device of appropriate type - dev = smi()->get_smart_device(name, type); - - if (!dev) { - pout("%s: %s\n", name, smi()->get_errmsg()); - if (type) - printvalidarglistmessage('d'); - else - pout("Smartctl: please specify device type with the -d option.\n"); + // define control block for external functions + smartmonctrl control; + con=&control; + + // Parse input arguments + ata_print_options ataopts; + scsi_print_options scsiopts; + const char * type = parse_options(argc, argv, ataopts, scsiopts); + + // '-d test' -> Report result of autodetection + bool print_type_only = (type && !strcmp(type, "test")); + if (print_type_only) + type = 0; + + const char * name = argv[argc-1]; + + smart_device_auto_ptr dev; + if (!strcmp(name,"-")) { + // Parse "smartctl -r ataioctl,2 ..." output from stdin + if (type || print_type_only) { + pout("Smartctl: -d option is not allowed in conjunction with device name \"-\".\n"); UsageSummary(); return FAILCMD; } + dev = get_parsed_ata_device(smi(), name); + } + else + // get device of appropriate type + dev = smi()->get_smart_device(name, type); + + if (!dev) { + pout("%s: %s\n", name, smi()->get_errmsg()); + if (type) + printvalidarglistmessage('d'); + else + pout("Smartctl: please specify device type with the -d option.\n"); + UsageSummary(); + return FAILCMD; + } - if (print_type_only) - // Report result of first autodetection - pout("%s: Device of type '%s' [%s] detected\n", - dev->get_info_name(), dev->get_dev_type(), get_protocol_info(dev)); - - // Open device - { - // Save old info - smart_device::device_info oldinfo = dev->get_info(); - - // Open with autodetect support, may return 'better' device - dev = dev->autodetect_open(); + if (print_type_only) + // Report result of first autodetection + pout("%s: Device of type '%s' [%s] detected\n", + dev->get_info_name(), dev->get_dev_type(), get_protocol_info(dev.get())); - // Report if type has changed - if ((type || print_type_only) && oldinfo.dev_type != dev->get_dev_type()) - pout("%s: Device open changed type from '%s' to '%s'\n", - dev->get_info_name(), oldinfo.dev_type.c_str(), dev->get_dev_type()); - } - if (!dev->is_open()) { - pout("Smartctl open device: %s failed: %s\n", dev->get_info_name(), dev->get_errmsg()); - delete dev; - return FAILDEV; - } + // Open device + { + // Save old info + smart_device::device_info oldinfo = dev->get_info(); - // now call appropriate ATA or SCSI routine - if (print_type_only) - pout("%s: Device of type '%s' [%s] opened\n", - dev->get_info_name(), dev->get_dev_type(), get_protocol_info(dev)); - else if (dev->is_ata()) - retval = ataPrintMain(dev->to_ata(), ataopts); - else if (dev->is_scsi()) - retval = scsiPrintMain(dev->to_scsi(), scsiopts); - else - // we should never fall into this branch! - pout("%s: Neither ATA nor SCSI device\n", dev->get_info_name()); + // Open with autodetect support, may return 'better' device + dev.replace( dev->autodetect_open() ); - dev->close(); - delete dev; + // Report if type has changed + if ((type || print_type_only) && oldinfo.dev_type != dev->get_dev_type()) + pout("%s: Device open changed type from '%s' to '%s'\n", + dev->get_info_name(), oldinfo.dev_type.c_str(), dev->get_dev_type()); } - catch (...) { - delete dev; - throw; + if (!dev->is_open()) { + pout("Smartctl open device: %s failed: %s\n", dev->get_info_name(), dev->get_errmsg()); + return FAILDEV; } + + // now call appropriate ATA or SCSI routine + int retval = 0; + if (print_type_only) + pout("%s: Device of type '%s' [%s] opened\n", + dev->get_info_name(), dev->get_dev_type(), get_protocol_info(dev.get())); + else if (dev->is_ata()) + retval = ataPrintMain(dev->to_ata(), ataopts); + else if (dev->is_scsi()) + retval = scsiPrintMain(dev->to_scsi(), scsiopts); + else + // we should never fall into this branch! + pout("%s: Neither ATA nor SCSI device\n", dev->get_info_name()); + + dev->close(); return retval; } diff --git a/smartmontools/smartd.cpp b/smartmontools/smartd.cpp index 3810edebd..b712f6702 100644 --- a/smartmontools/smartd.cpp +++ b/smartmontools/smartd.cpp @@ -4138,8 +4138,7 @@ static void RegisterDevices(const dev_config_vector & conf_entries, smart_device dev_config cfg = conf_entries[i]; // get device of appropriate type - // TODO: exception handling - smart_device * dev = 0; + smart_device_auto_ptr dev; bool scanning = false; // Device may already be detected during devicescan @@ -4164,10 +4163,10 @@ static void RegisterDevices(const dev_config_vector & conf_entries, smart_device smart_device::device_info oldinfo = dev->get_info(); // Open with autodetect support, may return 'better' device - dev = dev->autodetect_open(); + dev.replace( dev->autodetect_open() ); // Report if type has changed - if (/* ent->dev_type && */ oldinfo.dev_type != dev->get_dev_type()) + if (oldinfo.dev_type != dev->get_dev_type()) PrintOut(LOG_INFO,"Device: %s, type changed from '%s' to '%s'\n", cfg.name.c_str(), oldinfo.dev_type.c_str(), dev->get_dev_type()); @@ -4177,7 +4176,6 @@ static void RegisterDevices(const dev_config_vector & conf_entries, smart_device // If no debug and scanning - don't print errors if (debugmode || !scanning) PrintOut(LOG_INFO, "Device: %s, open() failed: %s\n", dev->get_info_name(), dev->get_errmsg()); - delete dev; continue; } @@ -4192,33 +4190,30 @@ static void RegisterDevices(const dev_config_vector & conf_entries, smart_device if (dev->is_ata()){ if (ATADeviceScan(cfg, state, dev->to_ata())) { CanNotRegister(cfg.name.c_str(), "ATA", cfg.lineno, scanning); - delete dev; dev = 0; - } - else { - // move onto the list of ata devices - configs.push_back(cfg); - states.push_back(state); - devices.push_back(dev); + dev.reset(); } } - // or register SCSI devices else if (dev->is_scsi()){ if (SCSIDeviceScan(cfg, state, dev->to_scsi())) { CanNotRegister(cfg.name.c_str(), "SCSI", cfg.lineno, scanning); - delete dev; dev = 0; - } - else { - // move onto the list of scsi devices - configs.push_back(cfg); - states.push_back(state); - devices.push_back(dev); + dev.reset(); } } - + else { + PrintOut(LOG_INFO, "Device: %s, neither ATA nor SCSI device\n", cfg.name.c_str()); + dev.reset(); + } + + if (dev) { + // move onto the list of devices + configs.push_back(cfg); + states.push_back(state); + devices.push_back(dev); + } // if device is explictly listed and we can't register it, then // exit unless the user has specified that the device is removable - if (!dev && !scanning) { + else if (!scanning) { if (cfg.removable || quit==2) PrintOut(LOG_INFO, "Device %s not available\n", cfg.name.c_str()); else { -- GitLab