some camcontrol(8) cleanups

From: Alexander Best <arundel_at_freebsd.org>
Date: Fri, 15 Oct 2010 20:47:48 +0000
hi there,

i sent this patch to mav_at_, 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 <arundel_at_freebsd.org> -----

Date: Mon, 27 Sep 2010 00:35:41 +0000
From: Alexander Best <arundel_at_freebsd.org>
To: Alexander Motin <mav_at_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
_at__at_ -116,7 +116,7 _at__at_ typedef enum {
 } cam_argmask;
 
 struct camcontrol_opts {
-	const char	*optname;	
+	const char	*optname;
 	cam_cmdmask	cmdnum;
 	cam_argmask	argnum;
 	const char	*subopt;
_at__at_ -204,7 +204,7 _at__at_ 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);
_at__at_ -234,7 +234,7 _at__at_ 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;
_at__at_ -622,7 +622,7 _at__at_ 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);
_at__at_ -688,7 +688,7 _at__at_ 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) {
_at__at_ -721,13 +721,13 _at__at_ 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,
_at__at_ -1181,7 +1181,7 _at__at_ 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");
_at__at_ -1201,16 +1201,13 _at__at_ 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");
_at__at_ -1223,28 +1220,39 _at__at_ 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");
_at__at_ -1255,7 +1263,6 _at__at_ 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)
 {
_at__at_ -1902,7 +1909,7 _at__at_ 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:
_at__at_ -2011,7 +2018,7 _at__at_ void
 reassignblocks(struct cam_device *device, u_int32_t *blocks, int num_blocks)
 {
 	union ccb *ccb;
-	
+
 	ccb = cam_getccb(device);
 
 	cam_freeccb(ccb);
_at__at_ -2114,7 +2121,7 _at__at_ 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);
_at__at_ -2294,7 +2301,7 _at__at_ 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;
_at__at_ -2611,7 +2618,7 _at__at_ 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;
_at__at_ -2874,7 +2881,7 _at__at_ 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)
_at__at_ -2913,7 +2920,7 _at__at_ 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)
_at__at_ -3764,9 +3771,9 _at__at_ 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);
_at__at_ -3956,7 +3963,7 _at__at_ 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)
_at__at_ -3994,7 +4001,7 _at__at_ 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",
_at__at_ -4352,7 +4359,7 _at__at_ bailout:
 
 #endif /* MINIMALISTIC */
 
-void 
+void
 usage(int verbose)
 {
 	fprintf(verbose ? stdout : stderr,
_at__at_ -4494,7 +4501,7 _at__at_ usage(int verbose)
 #endif /* MINIMALISTIC */
 }
 
-int 
+int
 main(int argc, char **argv)
 {
 	int c;
_at__at_ -4544,7 +4551,7 _at__at_ 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
_at__at_ -4553,13 +4560,13 _at__at_ 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
_at__at_ -4573,14 +4580,14 _at__at_ 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)
_at__at_ -4607,7 +4614,7 _at__at_ 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
Received on Fri Oct 15 2010 - 18:47:49 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:08 UTC