From owner-svn-src-head@freebsd.org Thu Apr 28 18:41:56 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B5767B1FE1C; Thu, 28 Apr 2016 18:41:56 +0000 (UTC) (envelope-from ngie@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 895B2124E; Thu, 28 Apr 2016 18:41:56 +0000 (UTC) (envelope-from ngie@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u3SIftMQ032792; Thu, 28 Apr 2016 18:41:55 GMT (envelope-from ngie@FreeBSD.org) Received: (from ngie@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u3SIftC2032791; Thu, 28 Apr 2016 18:41:55 GMT (envelope-from ngie@FreeBSD.org) Message-Id: <201604281841.u3SIftC2032791@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ngie set sender to ngie@FreeBSD.org using -f From: Garrett Cooper Date: Thu, 28 Apr 2016 18:41:55 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r298753 - head/lib/libcam X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Apr 2016 18:41:56 -0000 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)); }