svn commit: r298753 - head/lib/libcam

Garrett Cooper ngie at FreeBSD.org
Thu Apr 28 18:41:56 UTC 2016


Author: ngie
Date: Thu Apr 28 18:41:55 2016
New Revision: 298753
URL: https://svnweb.freebsd.org/changeset/base/298753

Log:
  Fix va_list handling
  
  - Add missing va_end's after corresponding va_start's to cleanup state
  - Eliminate questionable bzero'ing of va_list passed in to
    do_buff_decode(..) and do_encode(..) from buff_{de,en}code_visit(..)
    and csio_{de,en}code_visit(..). Make va_list a pointer instead and
    pass NULL into the underlying functions to handler this in a portable
    way.
  - Do some minor style(9) clean up in affected functions.
  
  Differential Revision: https://reviews.freebsd.org/D6072
  MFC after: 3 days
  Reported by: cppcheck, Coverity
  CID: 1018500-1018503
  Reviewed by: cem
  Sponsored by: EMC / Isilon Storage Division

Modified:
  head/lib/libcam/scsi_cmdparse.c

Modified: head/lib/libcam/scsi_cmdparse.c
==============================================================================
--- head/lib/libcam/scsi_cmdparse.c	Thu Apr 28 18:23:18 2016	(r298752)
+++ head/lib/libcam/scsi_cmdparse.c	Thu Apr 28 18:41:55 2016	(r298753)
@@ -102,7 +102,7 @@ __FBSDID("$FreeBSD$");
 static int
 do_buff_decode(u_int8_t *databuf, size_t len,
 	       void (*arg_put)(void *, int , void *, int, char *),
-	       void *puthook, const char *fmt, va_list ap)
+	       void *puthook, const char *fmt, va_list *ap)
 {
 	int assigned = 0;
 	int width;
@@ -128,7 +128,7 @@ do_buff_decode(u_int8_t *databuf, size_t
 					(void *)((long)(ARG)), width, \
 					field_name); \
 			else \
-				*(va_arg(ap, int *)) = (ARG); \
+				*(va_arg(*ap, int *)) = (ARG); \
 			assigned++; \
 		} \
 		field_name[0] = 0; \
@@ -255,7 +255,7 @@ do_buff_decode(u_int8_t *databuf, size_t
 						databuf, width, field_name);
 				else {
 					char *dest;
-					dest = va_arg(ap, char *);
+					dest = va_arg(*ap, char *);
 					bcopy(databuf, dest, width);
 					if (letter == 'z') {
 						char *p;
@@ -287,7 +287,7 @@ do_buff_decode(u_int8_t *databuf, size_t
 				 * can't have a variable seek when you are using
 				 * "arg_put".
 				 */
-				width = (arg_put) ? 0 : va_arg(ap, int);
+				width = (arg_put) ? 0 : va_arg(*ap, int);
 				fmt++;
 			} else {
 				width = strtol(fmt, &intendp, 10);
@@ -539,7 +539,7 @@ next_field(const char **pp, char *fmt, i
 static int
 do_encode(u_char *buff, size_t vec_max, size_t *used,
 	  int (*arg_get)(void *, char *), void *gethook, const char *fmt,
-	  va_list ap)
+	  va_list *ap)
 {
 	int ind;
 	int shift;
@@ -564,7 +564,7 @@ do_encode(u_char *buff, size_t vec_max, 
 			else
 				value = arg_get ?
 					(*arg_get)(gethook, field_name) :
-					va_arg(ap, int);
+					va_arg(*ap, int);
 		}
 
 #if 0
@@ -662,11 +662,16 @@ int
 csio_decode(struct ccb_scsiio *csio, const char *fmt, ...)
 {
 	va_list ap;
+	int retval;
 
 	va_start(ap, fmt);
 
-	return(do_buff_decode(csio->data_ptr, (size_t)csio->dxfer_len,
-			      0, 0, fmt, ap));
+	retval = do_buff_decode(csio->data_ptr, (size_t)csio->dxfer_len, 0, 0,
+		 fmt, &ap);
+
+	va_end(ap);
+
+	return (retval);
 }
 
 int
@@ -674,29 +679,31 @@ csio_decode_visit(struct ccb_scsiio *csi
 		  void (*arg_put)(void *, int, void *, int, char *),
 		  void *puthook)
 {
-	va_list ap;
 
 	/*
 	 * We need some way to output things; we can't do it without
 	 * the arg_put function.
 	 */
 	if (arg_put == NULL)
-		return(-1);
-
-	bzero(&ap, sizeof(ap));
+		return (-1);
 
-	return(do_buff_decode(csio->data_ptr, (size_t)csio->dxfer_len,
-			      arg_put, puthook, fmt, ap));
+	return (do_buff_decode(csio->data_ptr, (size_t)csio->dxfer_len,
+		    arg_put, puthook, fmt, NULL));
 }
 
 int
 buff_decode(u_int8_t *buff, size_t len, const char *fmt, ...)
 {
 	va_list ap;
+	int retval;
 
 	va_start(ap, fmt);
 
-	return(do_buff_decode(buff, len, 0, 0, fmt, ap));
+	retval = do_buff_decode(buff, len, 0, 0, fmt, &ap);
+
+	va_end(ap);
+
+	return (retval);
 }
 
 int
@@ -704,7 +711,6 @@ buff_decode_visit(u_int8_t *buff, size_t
 		  void (*arg_put)(void *, int, void *, int, char *),
 		  void *puthook)
 {
-	va_list ap;
 
 	/*
 	 * We need some way to output things; we can't do it without
@@ -713,9 +719,7 @@ buff_decode_visit(u_int8_t *buff, size_t
 	if (arg_put == NULL)
 		return(-1);
 
-	bzero(&ap, sizeof(ap));
-
-	return(do_buff_decode(buff, len, arg_put, puthook, fmt, ap));
+	return (do_buff_decode(buff, len, arg_put, puthook, fmt, NULL));
 }
 
 /*
@@ -732,15 +736,15 @@ csio_build(struct ccb_scsiio *csio, u_in
 	va_list ap;
 
 	if (csio == NULL)
-		return(0);
+		return (0);
 
 	bzero(csio, sizeof(struct ccb_scsiio));
 
 	va_start(ap, cmd_spec);
 
 	if ((retval = do_encode(csio->cdb_io.cdb_bytes, SCSI_MAX_CDBLEN,
-				&cmdlen, NULL, NULL, cmd_spec, ap)) == -1)
-		return(retval);
+				&cmdlen, NULL, NULL, cmd_spec, &ap)) == -1)
+		goto done;
 
 	cam_fill_csio(csio,
 		      /* retries */ retry_count,
@@ -753,7 +757,10 @@ csio_build(struct ccb_scsiio *csio, u_in
 		      /* cdb_len */ cmdlen,
 		      /* timeout */ timeout ? timeout : 5000);
 
-	return(retval);
+done:
+	va_end(ap);
+
+	return (retval);
 }
 
 int
@@ -762,7 +769,6 @@ csio_build_visit(struct ccb_scsiio *csio
 		 int timeout, const char *cmd_spec,
 		 int (*arg_get)(void *hook, char *field_name), void *gethook)
 {
-	va_list ap;
 	size_t cmdlen;
 	int retval;
 
@@ -776,12 +782,10 @@ csio_build_visit(struct ccb_scsiio *csio
 	if (arg_get == NULL)
 		return(-1);
 
-	bzero(&ap, sizeof(ap));
-
 	bzero(csio, sizeof(struct ccb_scsiio));
 
 	if ((retval = do_encode(csio->cdb_io.cdb_bytes, SCSI_MAX_CDBLEN,
-				&cmdlen, arg_get, gethook, cmd_spec, ap)) == -1)
+				&cmdlen, arg_get, gethook, cmd_spec, NULL)) == -1)
 		return(retval);
 
 	cam_fill_csio(csio,
@@ -802,20 +806,24 @@ int
 csio_encode(struct ccb_scsiio *csio, const char *fmt, ...)
 {
 	va_list ap;
+	int retval;
 
 	if (csio == NULL)
-		return(0);
+		return (0);
 
 	va_start(ap, fmt);
 
-	return(do_encode(csio->data_ptr, csio->dxfer_len, 0, 0, 0, fmt, ap));
+	retval = do_encode(csio->data_ptr, csio->dxfer_len, 0, 0, 0, fmt, &ap);
+
+	va_end(ap);
+
+	return (retval);
 }
 
 int
 buff_encode_visit(u_int8_t *buff, size_t len, const char *fmt,
 		  int (*arg_get)(void *hook, char *field_name), void *gethook)
 {
-	va_list ap;
 
 	/*
 	 * We need something to encode, but we can't get it without the
@@ -824,16 +832,13 @@ buff_encode_visit(u_int8_t *buff, size_t
 	if (arg_get == NULL)
 		return(-1);
 
-	bzero(&ap, sizeof(ap));
-
-	return(do_encode(buff, len, 0, arg_get, gethook, fmt, ap));
+	return (do_encode(buff, len, 0, arg_get, gethook, fmt, NULL));
 }
 
 int
 csio_encode_visit(struct ccb_scsiio *csio, const char *fmt,
 		  int (*arg_get)(void *hook, char *field_name), void *gethook)
 {
-	va_list ap;
 
 	/*
 	 * We need something to encode, but we can't get it without the
@@ -842,8 +847,6 @@ csio_encode_visit(struct ccb_scsiio *csi
 	if (arg_get == NULL)
 		return(-1);
 
-	bzero(&ap, sizeof(ap));
-
-	return(do_encode(csio->data_ptr, csio->dxfer_len, 0, arg_get,
-			 gethook, fmt, ap));
+	return (do_encode(csio->data_ptr, csio->dxfer_len, 0, arg_get,
+			 gethook, fmt, NULL));
 }


More information about the svn-src-head mailing list