hi there,
i sent this patch to mav@, but he seems rather busy atm.
maybe somebody else would like to take a look at it and see if it improves
camcontrol's current behavior.
cheers.
alex
----- Forwarded message from Alexander Best <arun...@freebsd.org> -----
Date: Mon, 27 Sep 2010 00:35:41 +0000
From: Alexander Best <arun...@freebsd.org>
To: Alexander Motin <m...@freebsd.org>
Subject: some camcontrol(8) cleanups
hi there,
since you're the most active committer to camcontrol i thought i'd mail you
this patch which does some whitespace cleaning up in camcontrol.c along with
some 'camcontrol identify' improvements (imo).
the only real change is that i removed this check:
if (parm->satacapabilities && parm->satacapabilities != 0xffff)
i've run the patched camcontrol against a few devices and none seemed to return
parm->satacapabilities == 0xffff.
so i don't think this check is needed in order to prevent 'camcontrol identify'
to falsely report NCQ supported/enabled.
cheers.
alex
--
a13x
diff --git a/sbin/camcontrol/camcontrol.c b/sbin/camcontrol/camcontrol.c
index 9f26906..b822282 100644
--- a/sbin/camcontrol/camcontrol.c
+++ b/sbin/camcontrol/camcontrol.c
@@ -116,7 +116,7 @@ typedef enum {
} cam_argmask;
struct camcontrol_opts {
- const char *optname;
+ const char *optname;
cam_cmdmask cmdnum;
cam_argmask argnum;
const char *subopt;
@@ -204,7 +204,7 @@ static int readdefects(struct cam_device *device, int argc,
char **argv,
char *combinedopt, int retry_count, int timeout);
static void modepage(struct cam_device *device, int argc, char **argv,
char *combinedopt, int retry_count, int timeout);
-static int scsicmd(struct cam_device *device, int argc, char **argv,
+static int scsicmd(struct cam_device *device, int argc, char **argv,
char *combinedopt, int retry_count, int timeout);
static int tagcontrol(struct cam_device *device, int argc, char **argv,
char *combinedopt);
@@ -234,7 +234,7 @@ static int atapm(struct cam_device *device, int argc, char
**argv,
#endif
camcontrol_optret
-getoption(char *arg, cam_cmdmask *cmdnum, cam_argmask *argnum,
+getoption(char *arg, cam_cmdmask *cmdnum, cam_argmask *argnum,
const char **subopt)
{
struct camcontrol_opts *opts;
@@ -622,7 +622,7 @@ scsistart(struct cam_device *device, int startstop, int
loadeject,
else
fprintf(stdout,
"Error received from stop unit command\n");
-
+
if (arglist & CAM_ARG_VERBOSE) {
cam_error_print(device, ccb, CAM_ESF_ALL,
CAM_EPF_ALL, stderr);
@@ -688,7 +688,7 @@ scsiinquiry(struct cam_device *device, int retry_count, int
timeout)
union ccb *ccb;
struct scsi_inquiry_data *inq_buf;
int error = 0;
-
+
ccb = cam_getccb(device);
if (ccb == NULL) {
@@ -721,13 +721,13 @@ scsiinquiry(struct cam_device *device, int retry_count,
int timeout)
* scsi_inquiry() will convert an inq_len (which is passed in as
* a u_int32_t, but the field in the CDB is only 1 byte) of 256
* to 0. Evidently, very few devices meet the spec in that
- * regard. Some devices, like many Seagate disks, take the 0 as
+ * regard. Some devices, like many Seagate disks, take the 0 as
* 0, and don't return any data. One Pioneer DVD-R drive
* returns more data than the command asked for.
*
* So, since there are numerous devices that just don't work
* right with the full inquiry size, we don't send the full size.
- *
+ *
* - The second reason not to use the full inquiry data length is
* that we don't need it here. The only reason we issue a
* standard inquiry is to get the vendor name, device name,
@@ -1181,7 +1181,7 @@ atacapprint(struct ata_params *parm)
}
printf("\nFeature "
- "Support Enable Value Vendor\n");
+ "Support Enabled Value Vendor\n");
printf("read ahead %s %s\n",
parm->support.command1 & ATA_SUPPORT_LOOKAHEAD ? "yes" : "no",
parm->enabled.command1 & ATA_SUPPORT_LOOKAHEAD ? "yes" : "no");
@@ -1201,16 +1201,13 @@ atacapprint(struct ata_params *parm)
ATA_QUEUE_LEN(parm->queue) + 1);
} else
printf("\n");
- if (parm->satacapabilities && parm->satacapabilities != 0xffff) {
- printf("Native Command Queuing (NCQ) %s ",
- parm->satacapabilities & ATA_SUPPORT_NCQ ?
- "yes" : "no");
+ printf("Native Command Queuing (NCQ) %s",
+ parm->satacapabilities & ATA_SUPPORT_NCQ ? "yes" : "no");
if (parm->satacapabilities & ATA_SUPPORT_NCQ) {
- printf(" %d tags\n",
+ printf(" %d tags\n",
ATA_QUEUE_LEN(parm->queue) + 1);
} else
printf("\n");
- }
printf("SMART %s %s\n",
parm->support.command1 & ATA_SUPPORT_SMART ? "yes" : "no",
parm->enabled.command1 & ATA_SUPPORT_SMART ? "yes" : "no");
@@ -1223,28 +1220,39 @@ atacapprint(struct ata_params *parm)
printf("power management %s %s\n",
parm->support.command1 & ATA_SUPPORT_POWERMGT ? "yes" : "no",
parm->enabled.command1 & ATA_SUPPORT_POWERMGT ? "yes" : "no");
- printf("advanced power management %s %s %d/0x%02X\n",
+ printf("advanced power management %s %s",
parm->support.command2 & ATA_SUPPORT_APM ? "yes" : "no",
- parm->enabled.command2 & ATA_SUPPORT_APM ? "yes" : "no",
- parm->apm_value, parm->apm_value);
- printf("automatic acoustic management %s %s "
- "%d/0x%02X %d/0x%02X\n",
+ parm->enabled.command2 & ATA_SUPPORT_APM ? "yes" : "no");
+ if (parm->support.command2 & ATA_SUPPORT_APM) {
+ printf(" %d/0x%02X\n",
+ parm->apm_value, parm->apm_value);
+ } else
+ printf("\n");
+ printf("automatic acoustic management %s %s",
parm->support.command2 & ATA_SUPPORT_AUTOACOUSTIC ? "yes" :"no",
- parm->enabled.command2 & ATA_SUPPORT_AUTOACOUSTIC ? "yes" :"no",
- ATA_ACOUSTIC_CURRENT(parm->acoustic),
- ATA_ACOUSTIC_CURRENT(parm->acoustic),
- ATA_ACOUSTIC_VENDOR(parm->acoustic),
- ATA_ACOUSTIC_VENDOR(parm->acoustic));
+ parm->enabled.command2 & ATA_SUPPORT_AUTOACOUSTIC ? "yes"
:"no");
+ if (parm->support.command2 & ATA_SUPPORT_AUTOACOUSTIC) {
+ printf(" %d/0x%02X %d/0x%02X\n",
+ ATA_ACOUSTIC_CURRENT(parm->acoustic),
+ ATA_ACOUSTIC_CURRENT(parm->acoustic),
+ ATA_ACOUSTIC_VENDOR(parm->acoustic),
+ ATA_ACOUSTIC_VENDOR(parm->acoustic));
+ } else
+ printf("\n");
printf("media status notification %s %s\n",
parm->support.command2 & ATA_SUPPORT_NOTIFY ? "yes" : "no",
parm->enabled.command2 & ATA_SUPPORT_NOTIFY ? "yes" : "no");
printf("power-up in Standby %s %s\n",
parm->support.command2 & ATA_SUPPORT_STANDBY ? "yes" : "no",
parm->enabled.command2 & ATA_SUPPORT_STANDBY ? "yes" : "no");
- printf("write-read-verify %s %s %d/0x%x\n",
+ printf("write-read-verify %s %s",
parm->support2 & ATA_SUPPORT_WRITEREADVERIFY ? "yes" : "no",
- parm->enabled2 & ATA_SUPPORT_WRITEREADVERIFY ? "yes" : "no",
- parm->wrv_mode, parm->wrv_mode);
+ parm->enabled2 & ATA_SUPPORT_WRITEREADVERIFY ? "yes" : "no");
+ if (parm->support2 & ATA_SUPPORT_WRITEREADVERIFY) {
+ printf(" %d/0x%x\n",
+ parm->wrv_mode, parm->wrv_mode);
+ } else
+ printf("\n");
printf("unload %s %s\n",
parm->support.extension & ATA_SUPPORT_UNLOAD ? "yes" : "no",
parm->enabled.extension & ATA_SUPPORT_UNLOAD ? "yes" : "no");
@@ -1255,7 +1263,6 @@ atacapprint(struct ata_params *parm)
parm->support_dsm & ATA_SUPPORT_DSM_TRIM ? "yes" : "no");
}
-
static int
ataidentify(struct cam_device *device, int retry_count, int timeout)
{
@@ -1902,7 +1909,7 @@ readdefects(struct cam_device *device, int argc, char
**argv,
/*
* XXX KDM I should probably clean up the printout format for the
- * disk defects.
+ * disk defects.
*/
switch (returned_format & SRDDH10_DLIST_FORMAT_MASK){
case SRDDH10_PHYSICAL_SECTOR_FORMAT:
@@ -2011,7 +2018,7 @@ void
reassignblocks(struct cam_device *device, u_int32_t *blocks, int num_blocks)
{
union ccb *ccb;
-
+
ccb = cam_getccb(device);
cam_freeccb(ccb);
@@ -2114,7 +2121,7 @@ mode_select(struct cam_device *device, int save_pages,
int retry_count,
err(1, "error sending mode select command");
else
errx(1, "error sending mode select command");
-
+
}
cam_freeccb(ccb);
@@ -2294,7 +2301,7 @@ scsicmd(struct cam_device *device, int argc, char **argv,
char *combinedopt,
if (arglist & CAM_ARG_CMD_IN) {
warnx("command must either be "
"read or write, not both");
- error = 1;
+ error = 1;
goto scsicmd_bailout;
}
arglist |= CAM_ARG_CMD_OUT;
@@ -2611,7 +2618,7 @@ camdebug(int argc, char **argv, char *combinedopt)
warnx("bus:target, or bus:target:lun to debug");
}
}
-
+
if (error == 0) {
ccb.ccb_h.func_code = XPT_DEBUG;
@@ -2874,7 +2881,7 @@ cts_print(struct cam_device *device, struct
ccb_trans_settings *cts)
}
/*
- * Get a path inquiry CCB for the specified device.
+ * Get a path inquiry CCB for the specified device.
*/
static int
get_cpi(struct cam_device *device, struct ccb_pathinq *cpi)
@@ -2913,7 +2920,7 @@ get_cpi_bailout:
}
/*
- * Get a get device CCB for the specified device.
+ * Get a get device CCB for the specified device.
*/
static int
get_cgd(struct cam_device *device, struct ccb_getdev *cgd)
@@ -3764,9 +3771,9 @@ doreport:
fprintf(stdout,
"\rFormatting: %ju.%02u %% "
"(%d/%d) done",
- (uintmax_t)(percentage /
+ (uintmax_t)(percentage /
(0x10000 * 100)),
- (unsigned)((percentage /
+ (unsigned)((percentage /
0x10000) % 100),
val, 0x10000);
fflush(stdout);
@@ -3956,7 +3963,7 @@ retry:
case RPL_LUNDATA_ATYP_PERIPH:
if ((lundata->luns[i].lundata[j] &
RPL_LUNDATA_PERIPH_BUS_MASK) != 0)
- fprintf(stdout, "%d:",
+ fprintf(stdout, "%d:",
lundata->luns[i].lundata[j] &
RPL_LUNDATA_PERIPH_BUS_MASK);
else if ((j == 0)
@@ -3994,7 +4001,7 @@ retry:
field_len_code = (lundata->luns[i].lundata[j] &
RPL_LUNDATA_EXT_LEN_MASK) >> 4;
field_len = field_len_code * 2;
-
+
if ((eam_code == RPL_LUNDATA_EXT_EAM_WK)
&& (field_len_code == 0x00)) {
fprintf(stdout, "%d",
@@ -4352,7 +4359,7 @@ bailout:
#endif /* MINIMALISTIC */
-void
+void
usage(int verbose)
{
fprintf(verbose ? stdout : stderr,
@@ -4494,7 +4501,7 @@ usage(int verbose)
#endif /* MINIMALISTIC */
}
-int
+int
main(int argc, char **argv)
{
int c;
@@ -4544,7 +4551,7 @@ main(int argc, char **argv)
* this. getopt is kinda braindead, so you end up having to run
* through the options twice, and give each invocation of getopt
* the option string for the other invocation.
- *
+ *
* You would think that you could just have two groups of options.
* The first group would get parsed by the first invocation of
* getopt, and the second group would get parsed by the second
@@ -4553,13 +4560,13 @@ main(int argc, char **argv)
* to the argument _after_ the first argument in the second group.
* So when the second invocation of getopt comes around, it doesn't
* recognize the first argument it gets and then bails out.
- *
+ *
* A nice alternative would be to have a flag for getopt that says
* "just keep parsing arguments even when you encounter an unknown
* argument", but there isn't one. So there's no real clean way to
* easily parse two sets of arguments without having one invocation
* of getopt know about the other.
- *
+ *
* Without this hack, the first invocation of getopt would work as
* long as the generic arguments are first, but the second invocation
* (in the subfunction) would fail in one of two ways. In the case
@@ -4573,14 +4580,14 @@ main(int argc, char **argv)
* whether optind had been incremented one option too far. The
* mechanics of that, however, are more daunting than just giving
* both invocations all of the expect options for either invocation.
- *
+ *
* Needless to say, I wouldn't mind if someone invented a better
* (non-GPL!) command line parsing interface than getopt. I
* wouldn't mind if someone added more knobs to getopt to make it
* work better. Who knows, I may talk myself into doing it someday,
* if the standards weenies let me. As it is, it just leads to
* hackery like this and causes people to avoid it in some cases.
- *
+ *
* KDM, September 8th, 1998
*/
if (subopt != NULL)
@@ -4607,7 +4614,7 @@ main(int argc, char **argv)
/*
* First catch people who try to do things like:
- * camcontrol tur /dev/da0
+ * camcontrol tur /dev/da0
* camcontrol doesn't take device nodes as arguments.
*/
if (argv[2][0] == '/') {
----- End forwarded message -----
--
a13x
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
