Re: svn commit: r249105 - in head/sys/cam: ata scsi
On Thu, 4 Apr 2013, Alexander Motin wrote: On 04.04.2013 23:53, Adrian Chadd wrote: Hi, Isn't this a prime candidate to replace with KASSERT()? It could be, but NULL dereference attempt will crash system no less reliably then KASSERT. Much more reliably: - if INAVRIANTS is not configured, then the NULL dereference still crashes properly - if INAVRIANTS is configured, then the NULL dereference gives a nice (restartable) fault, while KASSERT() calls panic() and there is no way to get back to the original context so as to to restart or debug it more easily. KASSERT() could be improved by replacing it by a a null dereference or other restartable fault, at least before calling panic() or taking any other unrestartable actions. The panic() call would still prevent restarting very easily -- you would have to back out to before the KASSERT() and fix up all the asserted conditions (usually more than a single null pointer). This method works well in userland too. Instead of assert() or abort(), use an null dereference, or more portably, a signal, or less portably, an asm with a breakpoint instruction or with the null pointer dereference (so that the compiler can't see that it gives undefined behaviour and optimize it away). I use this more to debug than to restart. Even if optimization or the debugger doesn't lose the local variables when assert() or abort() is called, it is easier to debug if you don't have to go up several frames to see the variables. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r249105 - in head/sys/cam: ata scsi
On Fri, Apr 5, 2013 at 8:21 AM, Bruce Evans b...@optusnet.com.au wrote: This method works well in userland too. Instead of assert() or abort(), use an null dereference, or more portably, a signal Digressing quite a bit, doesn't abort() send a signal already, i.e. SIGABRT? And doesn't __assert() call abort()? Cheers, matthew ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
Re: svn commit: r249105 - in head/sys/cam: ata scsi
On Fri, 5 Apr 2013 m...@freebsd.org wrote: On Fri, Apr 5, 2013 at 8:21 AM, Bruce Evans b...@optusnet.com.au wrote: This method works well in userland too. Instead of assert() or abort(), use an null dereference, or more portably, a signal Digressing quite a bit, doesn't abort() send a signal already, i.e. SIGABRT? And doesn't __assert() call abort()? Yes, but with assert() the signal occurs deeply nested in a function that doesn't return. Bruce ___ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org
svn commit: r249105 - in head/sys/cam: ata scsi
Author: mav
Date: Thu Apr 4 19:04:15 2013
New Revision: 249105
URL: http://svnweb.freebsd.org/changeset/base/249105
Log:
MFprojects/camlock r248930:
Remove extra NULL checks. d_drv1 can never be NULL during periph life cycle.
MFC after:2 weeks
Modified:
head/sys/cam/ata/ata_da.c
head/sys/cam/scsi/scsi_cd.c
head/sys/cam/scsi/scsi_da.c
Modified: head/sys/cam/ata/ata_da.c
==
--- head/sys/cam/ata/ata_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/ata/ata_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -527,10 +527,6 @@ adaopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return(ENXIO);
}
@@ -566,9 +562,6 @@ adaclose(struct disk *dp)
union ccb *ccb;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
cam_periph_unlock(periph);
@@ -646,10 +639,6 @@ adastrategy(struct bio *bp)
struct ada_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
@@ -704,8 +693,6 @@ adadump(void *arg, void *virtual, vm_off
dp = arg;
periph = dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
secsize = softc-params.secsize;
@@ -1038,9 +1025,6 @@ adagetattr(struct bio *bp)
struct cam_periph *periph;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
ret = xpt_getattr(bp-bio_data, bp-bio_length, bp-bio_attribute,
periph-path);
Modified: head/sys/cam/scsi/scsi_cd.c
==
--- head/sys/cam/scsi/scsi_cd.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_cd.c Thu Apr 4 19:04:15 2013(r249105)
@@ -386,7 +386,6 @@ cddiskgonecb(struct disk *dp)
struct cam_periph *periph;
periph = (struct cam_periph *)dp-d_drv1;
-
cam_periph_release(periph);
}
@@ -1073,9 +1072,6 @@ cdopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
if (cam_periph_acquire(periph) != CAM_REQ_CMP)
@@ -1120,9 +1116,6 @@ cdclose(struct disk *dp)
struct cd_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
cam_periph_lock(periph);
@@ -1473,11 +1466,6 @@ cdstrategy(struct bio *bp)
struct cd_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
-
cam_periph_lock(periph);
CAM_DEBUG(periph-path, CAM_DEBUG_TRACE,
(cdstrategy(%p)\n, bp));
@@ -1972,9 +1960,6 @@ cdioctl(struct disk *dp, u_long cmd, voi
int nocopyout, error = 0;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return(ENXIO);
-
cam_periph_lock(periph);
softc = (struct cd_softc *)periph-softc;
Modified: head/sys/cam/scsi/scsi_da.c
==
--- head/sys/cam/scsi/scsi_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -962,10 +962,6 @@ daopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return (ENXIO);
}
@@ -1027,9 +1023,6 @@ daclose(struct disk *dp)
struct da_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (0);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
cam_periph_unlock(periph);
@@ -1118,10 +,6 @@ dastrategy(struct bio *bp)
struct da_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
-
Re: svn commit: r249105 - in head/sys/cam: ata scsi
Hi,
Isn't this a prime candidate to replace with KASSERT()?
Thanks,
Adrian
On 4 April 2013 12:04, Alexander Motin m...@freebsd.org wrote:
Author: mav
Date: Thu Apr 4 19:04:15 2013
New Revision: 249105
URL: http://svnweb.freebsd.org/changeset/base/249105
Log:
MFprojects/camlock r248930:
Remove extra NULL checks. d_drv1 can never be NULL during periph life cycle.
MFC after:2 weeks
Modified:
head/sys/cam/ata/ata_da.c
head/sys/cam/scsi/scsi_cd.c
head/sys/cam/scsi/scsi_da.c
Modified: head/sys/cam/ata/ata_da.c
==
--- head/sys/cam/ata/ata_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/ata/ata_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -527,10 +527,6 @@ adaopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return(ENXIO);
}
@@ -566,9 +562,6 @@ adaclose(struct disk *dp)
union ccb *ccb;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
cam_periph_unlock(periph);
@@ -646,10 +639,6 @@ adastrategy(struct bio *bp)
struct ada_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
@@ -704,8 +693,6 @@ adadump(void *arg, void *virtual, vm_off
dp = arg;
periph = dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
secsize = softc-params.secsize;
@@ -1038,9 +1025,6 @@ adagetattr(struct bio *bp)
struct cam_periph *periph;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
ret = xpt_getattr(bp-bio_data, bp-bio_length, bp-bio_attribute,
periph-path);
Modified: head/sys/cam/scsi/scsi_cd.c
==
--- head/sys/cam/scsi/scsi_cd.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_cd.c Thu Apr 4 19:04:15 2013(r249105)
@@ -386,7 +386,6 @@ cddiskgonecb(struct disk *dp)
struct cam_periph *periph;
periph = (struct cam_periph *)dp-d_drv1;
-
cam_periph_release(periph);
}
@@ -1073,9 +1072,6 @@ cdopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
if (cam_periph_acquire(periph) != CAM_REQ_CMP)
@@ -1120,9 +1116,6 @@ cdclose(struct disk *dp)
struct cd_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
cam_periph_lock(periph);
@@ -1473,11 +1466,6 @@ cdstrategy(struct bio *bp)
struct cd_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
-
cam_periph_lock(periph);
CAM_DEBUG(periph-path, CAM_DEBUG_TRACE,
(cdstrategy(%p)\n, bp));
@@ -1972,9 +1960,6 @@ cdioctl(struct disk *dp, u_long cmd, voi
int nocopyout, error = 0;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return(ENXIO);
-
cam_periph_lock(periph);
softc = (struct cd_softc *)periph-softc;
Modified: head/sys/cam/scsi/scsi_da.c
==
--- head/sys/cam/scsi/scsi_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -962,10 +962,6 @@ daopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return (ENXIO);
}
@@ -1027,9 +1023,6 @@ daclose(struct disk *dp)
struct da_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (0);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
cam_periph_unlock(periph);
@@ -1118,10
Re: svn commit: r249105 - in head/sys/cam: ata scsi
On 04.04.2013 23:53, Adrian Chadd wrote:
Hi,
Isn't this a prime candidate to replace with KASSERT()?
It could be, but NULL dereference attempt will crash system no less
reliably then KASSERT.
On 4 April 2013 12:04, Alexander Motin m...@freebsd.org wrote:
Author: mav
Date: Thu Apr 4 19:04:15 2013
New Revision: 249105
URL: http://svnweb.freebsd.org/changeset/base/249105
Log:
MFprojects/camlock r248930:
Remove extra NULL checks. d_drv1 can never be NULL during periph life cycle.
MFC after:2 weeks
Modified:
head/sys/cam/ata/ata_da.c
head/sys/cam/scsi/scsi_cd.c
head/sys/cam/scsi/scsi_da.c
Modified: head/sys/cam/ata/ata_da.c
==
--- head/sys/cam/ata/ata_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/ata/ata_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -527,10 +527,6 @@ adaopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return(ENXIO);
}
@@ -566,9 +562,6 @@ adaclose(struct disk *dp)
union ccb *ccb;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
cam_periph_unlock(periph);
@@ -646,10 +639,6 @@ adastrategy(struct bio *bp)
struct ada_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
@@ -704,8 +693,6 @@ adadump(void *arg, void *virtual, vm_off
dp = arg;
periph = dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
softc = (struct ada_softc *)periph-softc;
cam_periph_lock(periph);
secsize = softc-params.secsize;
@@ -1038,9 +1025,6 @@ adagetattr(struct bio *bp)
struct cam_periph *periph;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
cam_periph_lock(periph);
ret = xpt_getattr(bp-bio_data, bp-bio_length, bp-bio_attribute,
periph-path);
Modified: head/sys/cam/scsi/scsi_cd.c
==
--- head/sys/cam/scsi/scsi_cd.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_cd.c Thu Apr 4 19:04:15 2013(r249105)
@@ -386,7 +386,6 @@ cddiskgonecb(struct disk *dp)
struct cam_periph *periph;
periph = (struct cam_periph *)dp-d_drv1;
-
cam_periph_release(periph);
}
@@ -1073,9 +1072,6 @@ cdopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
if (cam_periph_acquire(periph) != CAM_REQ_CMP)
@@ -1120,9 +1116,6 @@ cdclose(struct disk *dp)
struct cd_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (ENXIO);
-
softc = (struct cd_softc *)periph-softc;
cam_periph_lock(periph);
@@ -1473,11 +1466,6 @@ cdstrategy(struct bio *bp)
struct cd_softc *softc;
periph = (struct cam_periph *)bp-bio_disk-d_drv1;
- if (periph == NULL) {
- biofinish(bp, NULL, ENXIO);
- return;
- }
-
cam_periph_lock(periph);
CAM_DEBUG(periph-path, CAM_DEBUG_TRACE,
(cdstrategy(%p)\n, bp));
@@ -1972,9 +1960,6 @@ cdioctl(struct disk *dp, u_long cmd, voi
int nocopyout, error = 0;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return(ENXIO);
-
cam_periph_lock(periph);
softc = (struct cd_softc *)periph-softc;
Modified: head/sys/cam/scsi/scsi_da.c
==
--- head/sys/cam/scsi/scsi_da.c Thu Apr 4 18:59:29 2013(r249104)
+++ head/sys/cam/scsi/scsi_da.c Thu Apr 4 19:04:15 2013(r249105)
@@ -962,10 +962,6 @@ daopen(struct disk *dp)
int error;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL) {
- return (ENXIO);
- }
-
if (cam_periph_acquire(periph) != CAM_REQ_CMP) {
return (ENXIO);
}
@@ -1027,9 +1023,6 @@ daclose(struct disk *dp)
struct da_softc *softc;
periph = (struct cam_periph *)dp-d_drv1;
- if (periph == NULL)
- return (0);
-
cam_periph_lock(periph);
if (cam_periph_hold(periph, PRIBIO) != 0) {
