From owner-svn-src-all@FreeBSD.ORG Wed Oct 10 22:02:12 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 59B67EE8; Wed, 10 Oct 2012 22:02:12 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 431F08FC17; Wed, 10 Oct 2012 22:02:12 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q9AM2CRB010155; Wed, 10 Oct 2012 22:02:12 GMT (envelope-from mav@svn.freebsd.org) Received: (from mav@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q9AM2CiQ010153; Wed, 10 Oct 2012 22:02:12 GMT (envelope-from mav@svn.freebsd.org) Message-Id: <201210102202.q9AM2CiQ010153@svn.freebsd.org> From: Alexander Motin Date: Wed, 10 Oct 2012 22:02:12 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r241432 - head/sys/dev/usb/storage X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Oct 2012 22:02:12 -0000 Author: mav Date: Wed Oct 10 22:02:11 2012 New Revision: 241432 URL: http://svn.freebsd.org/changeset/base/241432 Log: - Remove ancient checks for sim->softc == NULL. It can't be NULL, as it is set not-NULL during SIM registration and set to UMASS_GONE on destruction. Debug messages there look broken for at least 9 years, as they dereference softc value that was just checked to be equal to NULL. - Remove magic pointer value UMASS_GONE and use simple NULL instead. Found by: Clang Static Analyzer Modified: head/sys/dev/usb/storage/umass.c Modified: head/sys/dev/usb/storage/umass.c ============================================================================== --- head/sys/dev/usb/storage/umass.c Wed Oct 10 21:38:17 2012 (r241431) +++ head/sys/dev/usb/storage/umass.c Wed Oct 10 22:02:11 2012 (r241432) @@ -175,8 +175,6 @@ TUNABLE_INT("hw.usb.umass.debug", &umass #define DPRINTF(...) do { } while (0) #endif -#define UMASS_GONE ((struct umass_softc *)1) - #define UMASS_BULK_SIZE (1 << 17) #define UMASS_CBI_DIAGNOSTIC_CMDLEN 12 /* bytes */ #define UMASS_MAX_CMDLEN MAX(12, CAM_MAX_CDBLEN) /* bytes */ @@ -2109,7 +2107,7 @@ umass_cam_detach_sim(struct umass_softc if (sc->sc_sim != NULL) { if (xpt_bus_deregister(cam_sim_path(sc->sc_sim))) { /* accessing the softc is not possible after this */ - sc->sc_sim->softc = UMASS_GONE; + sc->sc_sim->softc = NULL; cam_sim_free(sc->sc_sim, /* free_devq */ TRUE); } else { panic("%s: CAM layer is busy\n", @@ -2128,63 +2126,11 @@ umass_cam_action(struct cam_sim *sim, un { struct umass_softc *sc = (struct umass_softc *)sim->softc; - if (sc == UMASS_GONE || - (sc != NULL && !usbd_device_attached(sc->sc_udev))) { + if (sc == NULL) { ccb->ccb_h.status = CAM_SEL_TIMEOUT; xpt_done(ccb); return; } - /* - * Verify, depending on the operation to perform, that we either got - * a valid sc, because an existing target was referenced, or - * otherwise the SIM is addressed. - * - * This avoids bombing out at a printf and does give the CAM layer some - * sensible feedback on errors. - */ - switch (ccb->ccb_h.func_code) { - case XPT_SCSI_IO: - case XPT_RESET_DEV: - case XPT_GET_TRAN_SETTINGS: - case XPT_SET_TRAN_SETTINGS: - case XPT_CALC_GEOMETRY: - /* the opcodes requiring a target. These should never occur. */ - if (sc == NULL) { - DPRINTF(sc, UDMASS_GEN, "%s:%d:%d:%d:func_code 0x%04x: " - "Invalid target (target needed)\n", - DEVNAME_SIM, cam_sim_path(sc->sc_sim), - ccb->ccb_h.target_id, ccb->ccb_h.target_lun, - ccb->ccb_h.func_code); - - ccb->ccb_h.status = CAM_TID_INVALID; - xpt_done(ccb); - goto done; - } - break; - case XPT_PATH_INQ: - case XPT_NOOP: - /* - * The opcodes sometimes aimed at a target (sc is valid), - * sometimes aimed at the SIM (sc is invalid and target is - * CAM_TARGET_WILDCARD) - */ - if ((sc == NULL) && - (ccb->ccb_h.target_id != CAM_TARGET_WILDCARD)) { - DPRINTF(sc, UDMASS_SCSI, "%s:%d:%d:%d:func_code 0x%04x: " - "Invalid target (no wildcard)\n", - DEVNAME_SIM, cam_sim_path(sc->sc_sim), - ccb->ccb_h.target_id, ccb->ccb_h.target_lun, - ccb->ccb_h.func_code); - - ccb->ccb_h.status = CAM_TID_INVALID; - xpt_done(ccb); - goto done; - } - break; - default: - /* XXX Hm, we should check the input parameters */ - break; - } /* Perform the requested action */ switch (ccb->ccb_h.func_code) { @@ -2448,7 +2394,7 @@ umass_cam_poll(struct cam_sim *sim) { struct umass_softc *sc = (struct umass_softc *)sim->softc; - if (sc == UMASS_GONE) + if (sc == NULL) return; DPRINTF(sc, UDMASS_SCSI, "CAM poll\n");