Index: usr.sbin/ctladm/ctladm.c =================================================================== --- usr.sbin/ctladm/ctladm.c +++ usr.sbin/ctladm/ctladm.c @@ -351,7 +351,6 @@ ooa.alloc_num = num_entries; if (ioctl(fd, CTL_GET_OOA, &ooa) == -1) { warn("%s: CTL_GET_OOA ioctl failed", __func__); - retval = 1; goto bailout; } @@ -365,7 +364,6 @@ if (ooa.status != CTL_OOA_OK) { warnx("%s: CTL_GET_OOA ioctl returned error %d", __func__, ooa.status); - retval = 1; goto bailout; } @@ -781,8 +779,8 @@ char *combinedopt) { struct ctl_io_delay_info delay_info; - char *delayloc = NULL; - char *delaytype = NULL; + const char *delayloc = NULL; + const char *delaytype = NULL; int delaytime = -1; int retval; int c; @@ -794,10 +792,10 @@ while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'T': - delaytype = strdup(optarg); + delaytype = optarg; break; case 'l': - delayloc = strdup(optarg); + delayloc = optarg; break; case 't': delaytime = strtoul(optarg, NULL, 0); @@ -873,12 +871,8 @@ retval = 1; break; } -bailout: - - /* delayloc should never be NULL, but just in case...*/ - if (delayloc != NULL) - free(delayloc); +bailout: return (retval); } @@ -1098,35 +1092,35 @@ break; } case 'r': { - char *tmpstr, *tmpstr2; + char *token, *string, *tofree; - tmpstr = strdup(optarg); - if (tmpstr == NULL) { + string = tofree = strdup(optarg); + if (tofree == NULL) { warn("%s: error duplicating string %s", __func__, optarg); retval = 1; goto bailout; } - tmpstr2 = strsep(&tmpstr, ","); - if (tmpstr2 == NULL) { + token = strsep(&string, ","); + if (token == NULL) { warnx("%s: invalid -r argument %s", __func__, optarg); retval = 1; - free(tmpstr); + free(tofree); goto bailout; } - lba = strtoull(tmpstr2, NULL, 0); - tmpstr2 = strsep(&tmpstr, ","); - if (tmpstr2 == NULL) { + lba = strtoull(token, NULL, 0); + token = strsep(&string, ","); + if (token == NULL) { warnx("%s: no len argument for -r lba,len, got" " %s", __func__, optarg); retval = 1; - free(tmpstr); + free(tofree); goto bailout; } - len = strtoul(tmpstr2, NULL, 0); - free(tmpstr); + len = strtoul(string, NULL, 0); + free(tofree); break; } case 's': { @@ -1262,7 +1256,6 @@ unsigned int i; int retval; - retval = 0; inq_data = NULL; target = 6; @@ -1272,8 +1265,9 @@ * XXX KDM assuming LUN 0 is fine, but we may need to change this * if we ever acquire the ability to have multiple targets. */ - if ((retval = cctl_get_luns(fd, target, /*lun*/ 0, initid, - /*retries*/ 2, &lun_data, &num_luns)) != 0) + retval = cctl_get_luns(fd, target, /*lun*/ 0, initid, + /*retries*/ 2, &lun_data, &num_luns); + if (retval != 0) goto bailout; inq_data = malloc(sizeof(*inq_data)); @@ -1309,12 +1303,12 @@ if (lun_val == -1) continue; - if ((retval = cctl_get_inquiry(fd, target, lun_val, initid, - /*retries*/ 2, scsi_path, - sizeof(scsi_path), - inq_data)) != 0) { + retval = cctl_get_inquiry(fd, target, lun_val, initid, + /*retries*/ 2, scsi_path, + sizeof(scsi_path), + inq_data); + if (retval != 0) goto bailout; - } printf("%s", scsi_path); scsi_print_inquiry(inq_data); } @@ -1341,7 +1335,6 @@ unsigned int i; int retval; - retval = 0; inq_data = NULL; /* @@ -1362,8 +1355,9 @@ return (1); } - if ((retval = cctl_get_luns(fd, target, lun, iid, /*retries*/ 2, - &lun_data, &num_luns)) != 0) + retval = cctl_get_luns(fd, target, lun, iid, /*retries*/ 2, + &lun_data, &num_luns); + if (retval != 0) goto bailout; inq_data = malloc(sizeof(*inq_data)); @@ -1403,12 +1397,11 @@ if (lun_val == -1) continue; - if ((retval = cctl_get_inquiry(fd, target, lun_val, iid, - /*retries*/ 2, scsi_path, - sizeof(scsi_path), - inq_data)) != 0) { + retval = cctl_get_inquiry(fd, target, lun_val, iid, + /*retries*/ 2, scsi_path, sizeof(scsi_path), + inq_data); + if (retval != 0) goto bailout; - } printf("%s", scsi_path); scsi_print_inquiry(inq_data); /* @@ -1770,15 +1763,13 @@ else datalen = 65535; - dataptr = (uint8_t *)malloc(datalen); + dataptr = calloc(1, datalen); if (dataptr == NULL) { warn("%s: can't allocate %d bytes", __func__, datalen); retval = 1; goto bailout; } - memset(dataptr, 0, datalen); - ctl_scsi_mode_sense(io, /*data_ptr*/ dataptr, /*data_len*/ datalen, @@ -1910,14 +1901,13 @@ } else cdbsize = 10; - dataptr = (uint8_t *)malloc(sizeof(*longdata)); + dataptr = calloc(1, sizeof(*longdata)); if (dataptr == NULL) { warn("%s: can't allocate %zd bytes\n", __func__, sizeof(*longdata)); retval = 1; goto bailout; } - memset(dataptr, 0, sizeof(*longdata)); retry: @@ -1999,7 +1989,7 @@ int file_fd, do_stdio; int cdbsize = -1, databytes; uint8_t *dataptr; - char *filename = NULL; + const char *filename = NULL; int datalen = -1, blocksize = -1; uint64_t lba = 0; int lba_set = 0; @@ -2033,7 +2023,7 @@ datalen = strtoul(optarg, NULL, 0); break; case 'f': - filename = strdup(optarg); + filename = optarg; break; case 'l': lba = strtoull(optarg, NULL, 0); @@ -2085,7 +2075,7 @@ cdbsize = 6; databytes = datalen * blocksize; - dataptr = (uint8_t *)malloc(databytes); + dataptr = calloc(datalen, blocksize); if (dataptr == NULL) { warn("%s: can't allocate %d bytes\n", __func__, databytes); @@ -2107,8 +2097,6 @@ } } - memset(dataptr, 0, databytes); - if (command == CTLADM_CMD_WRITE) { int bytes_read; @@ -2254,8 +2242,9 @@ lun_data = NULL; - if ((retval = cctl_get_luns(fd, target, lun, iid, retries, &lun_data, - &num_luns)) != 0) + retval = cctl_get_luns(fd, target, lun, iid, retries, &lun_data, + &num_luns); + if (retval != 0) goto bailout; fprintf(stdout, "%u LUNs returned\n", num_luns); @@ -2388,8 +2377,6 @@ char scsi_path[40]; int retval; - retval = 0; - inq_data = malloc(sizeof(*inq_data)); if (inq_data == NULL) { warnx("%s: can't allocate inquiry data", __func__); @@ -2397,8 +2384,9 @@ goto bailout; } - if ((retval = cctl_get_inquiry(fd, target, lun, iid, retries, scsi_path, - sizeof(scsi_path), inq_data)) != 0) + retval = cctl_get_inquiry(fd, target, lun, iid, retries, + scsi_path, sizeof(scsi_path), inq_data); + if (retval != 0) goto bailout; printf("%s", scsi_path); @@ -2428,8 +2416,7 @@ warnx("cctl_req_sense: can't allocate memory\n"); return (1); } - sense_data = malloc(sizeof(*sense_data)); - memset(sense_data, 0, sizeof(*sense_data)); + sense_data = calloc(1, sizeof(*sense_data)); ctl_scsi_request_sense(/*io*/ io, /*data_ptr*/ (uint8_t *)sense_data, @@ -2482,15 +2469,13 @@ } datalen = 64; - dataptr = (uint8_t *)malloc(datalen); + dataptr = calloc(1, datalen); if (dataptr == NULL) { warn("%s: can't allocate %d bytes", __func__, datalen); retval = 1; goto bailout; } - memset(dataptr, 0, datalen); - ctl_scsi_maintenance_in(/*io*/ io, /*data_ptr*/ dataptr, /*data_len*/ datalen, @@ -2550,15 +2535,13 @@ } datalen = 256; - dataptr = (uint8_t *)malloc(datalen); + dataptr = calloc(1, datalen); if (dataptr == NULL) { warn("%s: can't allocate %d bytes", __func__, datalen); retval = 1; goto bailout; } - memset(dataptr, 0, datalen); - ctl_scsi_inquiry(/*io*/ io, /*data_ptr*/ dataptr, /*data_len*/ datalen, @@ -2640,15 +2623,13 @@ datalen = 256; - dataptr = (uint8_t *)malloc(datalen); + dataptr = calloc(1, datalen); if (dataptr == NULL) { warn("%s: can't allocate %d bytes", __func__, datalen); retval = 1; goto bailout; } - memset(dataptr, 0, datalen); - ctl_scsi_persistent_res_in(io, /*data_ptr*/ dataptr, /*data_len*/ datalen, @@ -2762,15 +2743,13 @@ } datalen = 24; - dataptr = (uint8_t *)malloc(datalen); + dataptr = calloc(1, datalen); if (dataptr == NULL) { warn("%s: can't allocate %d bytes", __func__, datalen); retval = 1; goto bailout; } - memset(dataptr, 0, datalen); - ctl_scsi_persistent_res_out(io, /*data_ptr*/ dataptr, /*data_len*/ datalen, @@ -2821,50 +2800,59 @@ int device_type = -1; uint64_t lun_size = 0; uint32_t blocksize = 0, req_lun_id = 0; - char *serial_num = NULL; - char *device_id = NULL; + const char *serial_num = NULL; + const char *device_id = NULL; int lun_size_set = 0, blocksize_set = 0, lun_id_set = 0; - char *backend_name = NULL; + const char *backend_name = NULL; STAILQ_HEAD(, cctl_req_option) option_list; int num_options = 0; int retval = 0, c; + struct cctl_req_option *option, *next_option; STAILQ_INIT(&option_list); while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'b': - backend_name = strdup(optarg); + backend_name = optarg; break; case 'B': blocksize = strtoul(optarg, NULL, 0); blocksize_set = 1; break; case 'd': - device_id = strdup(optarg); + device_id = optarg; break; case 'l': req_lun_id = strtoul(optarg, NULL, 0); lun_id_set = 1; break; case 'o': { - struct cctl_req_option *option; - char *tmpstr; - char *name, *value; + char *string, *tofree; + const char *name, *value; + + string = tofree = strdup(optarg); + if (tofree == NULL) { + warn("%s: error duplicating string %s", + __func__, optarg); + retval = 1; + goto bailout; + } - tmpstr = strdup(optarg); - name = strsep(&tmpstr, "="); + name = strsep(&string, "="); if (name == NULL) { warnx("%s: option -o takes \"name=value\"" "argument", __func__); retval = 1; + free(tofree); goto bailout; } - value = strsep(&tmpstr, "="); + value = strsep(&string, "="); if (value == NULL) { warnx("%s: option -o takes \"name=value\"" "argument", __func__); retval = 1; + free(tofree); goto bailout; } option = malloc(sizeof(*option)); @@ -2872,13 +2860,14 @@ warn("%s: error allocating %zd bytes", __func__, sizeof(*option)); retval = 1; + free(tofree); goto bailout; } option->name = strdup(name); option->namelen = strlen(name) + 1; option->value = strdup(value); option->vallen = strlen(value) + 1; - free(tmpstr); + free(tofree); STAILQ_INSERT_TAIL(&option_list, option, links); num_options++; @@ -2897,7 +2886,7 @@ lun_size_set = 1; break; case 'S': - serial_num = strdup(optarg); + serial_num = optarg; break; case 't': device_type = strtoul(optarg, NULL, 0); @@ -2950,7 +2939,6 @@ req.num_be_args = num_options; if (num_options > 0) { - struct cctl_req_option *option, *next_option; int i; req.be_args = malloc(num_options * sizeof(*req.be_args)); @@ -3018,6 +3006,16 @@ fprintf(stdout, "Device ID; %s\n", req.reqdata.create.device_id); bailout: + if (!STAILQ_EMPTY(&option_list)) { + STAILQ_FOREACH_SAFE(option, &option_list, + links, next_option) { + STAILQ_REMOVE(&option_list, option, cctl_req_option, + links); + free(option->name); + free(option->value); + free(option); + } + } return (retval); } @@ -3027,40 +3025,48 @@ struct ctl_lun_req req; uint32_t lun_id = 0; int lun_id_set = 0; - char *backend_name = NULL; + const char *backend_name = NULL; STAILQ_HEAD(, cctl_req_option) option_list; int num_options = 0; int retval = 0, c; + struct cctl_req_option *option, *next_option; STAILQ_INIT(&option_list); while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'b': - backend_name = strdup(optarg); + backend_name = optarg; break; case 'l': lun_id = strtoul(optarg, NULL, 0); lun_id_set = 1; break; case 'o': { - struct cctl_req_option *option; - char *tmpstr; - char *name, *value; + char *string, *tofree; + const char *name, *value; - tmpstr = strdup(optarg); - name = strsep(&tmpstr, "="); + string = tofree = strdup(optarg); + if (tofree == NULL) { + warn("%s: error duplicating string %s", + __func__, optarg); + retval = 1; + goto bailout; + } + name = strsep(&string, "="); if (name == NULL) { warnx("%s: option -o takes \"name=value\"" "argument", __func__); retval = 1; + free(tofree); goto bailout; } - value = strsep(&tmpstr, "="); + value = strsep(&string, "="); if (value == NULL) { warnx("%s: option -o takes \"name=value\"" "argument", __func__); retval = 1; + free(tofree); goto bailout; } option = malloc(sizeof(*option)); @@ -3068,13 +3074,14 @@ warn("%s: error allocating %zd bytes", __func__, sizeof(*option)); retval = 1; + free(tofree); goto bailout; } option->name = strdup(name); option->namelen = strlen(name) + 1; option->value = strdup(value); option->vallen = strlen(value) + 1; - free(tmpstr); + free(tofree); STAILQ_INSERT_TAIL(&option_list, option, links); num_options++; @@ -3100,7 +3107,6 @@ req.num_be_args = num_options; if (num_options > 0) { - struct cctl_req_option *option, *next_option; int i; req.be_args = malloc(num_options * sizeof(*req.be_args)); @@ -3159,6 +3165,16 @@ printf("LUN %d removed successfully\n", lun_id); bailout: + if (!STAILQ_EMPTY(&option_list)) { + STAILQ_FOREACH_SAFE(option, &option_list, + links, next_option) { + STAILQ_REMOVE(&option_list, option, cctl_req_option, + links); + free(option->name); + free(option->value); + free(option); + } + } return (retval); } @@ -3169,13 +3185,13 @@ uint64_t lun_size = 0; uint32_t lun_id = 0; int lun_id_set = 0, lun_size_set = 0; - char *backend_name = NULL; + const char *backend_name = NULL; int retval = 0, c; while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'b': - backend_name = strdup(optarg); + backend_name = optarg; break; case 'l': lun_id = strtoul(optarg, NULL, 0); @@ -3515,7 +3531,7 @@ struct ctl_iscsi req; int retval = 0, c; int all = 0, connection_id = -1, nargs = 0; - char *initiator_name = NULL, *initiator_addr = NULL; + const char *initiator_name = NULL, *initiator_addr = NULL; while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { @@ -3528,15 +3544,11 @@ nargs++; break; case 'i': - initiator_name = strdup(optarg); - if (initiator_name == NULL) - err(1, "%s: strdup", __func__); + initiator_name = optarg; nargs++; break; case 'p': - initiator_addr = strdup(optarg); - if (initiator_addr == NULL) - err(1, "%s: strdup", __func__); + initiator_addr = optarg; nargs++; break; default: @@ -3588,7 +3600,7 @@ struct ctl_iscsi req; int retval = 0, c; int all = 0, connection_id = -1, nargs = 0; - char *initiator_name = NULL, *initiator_addr = NULL; + const char *initiator_name = NULL, *initiator_addr = NULL; while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { @@ -3601,15 +3613,11 @@ nargs++; break; case 'i': - initiator_name = strdup(optarg); - if (initiator_name == NULL) - err(1, "%s: strdup", __func__); + initiator_name = optarg; nargs++; break; case 'p': - initiator_addr = strdup(optarg); - if (initiator_addr == NULL) - err(1, "%s: strdup", __func__); + initiator_addr = optarg; nargs++; break; default: @@ -3824,7 +3832,7 @@ int lun_len; int dump_xml = 0; int retval, c; - char *backend = NULL; + const char *backend = NULL; int verbose = 0; retval = 0; @@ -3836,7 +3844,7 @@ while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'b': - backend = strdup(optarg); + backend = optarg; break; case 'v': verbose++; @@ -4108,7 +4116,7 @@ int port_len; int dump_xml = 0; int retval, c; - char *frontend = NULL; + const char *frontend = NULL; uint64_t portarg = UINT64_MAX; int verbose = 0, init = 0, lun = 0, quiet = 0; @@ -4121,7 +4129,7 @@ while ((c = getopt(argc, argv, combinedopt)) != -1) { switch (c) { case 'f': - frontend = strdup(optarg); + frontend = optarg; break; case 'i': init++; @@ -4432,7 +4440,7 @@ ctladm_cmdfunction command; ctladm_cmdargs cmdargs; ctladm_optret optreturn; - char *device; + const char *device = NULL; const char *mainopt = "C:D:I:"; const char *subopt = NULL; char combinedopt[256]; @@ -4446,7 +4454,6 @@ retval = 0; cmdargs = CTLADM_ARG_NONE; command = CTLADM_CMD_HELP; - device = NULL; fd = -1; retries = 0; target = 0; @@ -4561,7 +4568,7 @@ retries = strtol(optarg, NULL, 0); break; case 'D': - device = strdup(optarg); + device = optarg; cmdargs |= CTLADM_ARG_DEVICE; break; case 'I': @@ -4584,7 +4591,7 @@ */ if (((cmdargs & CTLADM_ARG_DEVICE) == 0) && (command != CTLADM_CMD_HELP)) { - device = strdup(CTL_DEFAULT_DEV); + device = CTL_DEFAULT_DEV; cmdargs |= CTLADM_ARG_DEVICE; }