This structure isn't needed. Passing arguments to display_* routines makes the code more readable.
The only thing the display_* options need are the ifname and display_target_info() is used for both LUN and target displaying so it needs a flag. An enum was created for the second case. The interface name can just be a pointer that's assigned to the appropriate argv element since that memory will not be free'd durring fcoeadm's execution. This makes things nicer too becuase now we don't need to check string sizes to see if an ifname was provided. Now we can just check if the pointer is NULL or not. Signed-off-by: Robert Love <[email protected]> --- fcoe_utils.c | 2 + fcoeadm.c | 77 +++++++++++++++++++-------------------------------- fcoeadm.h | 26 +++++++---------- fcoeadm_display.c | 80 ++++++++++++++++++++++------------------------------- 4 files changed, 72 insertions(+), 113 deletions(-) diff --git a/fcoe_utils.c b/fcoe_utils.c index db62822..506356d 100644 --- a/fcoe_utils.c +++ b/fcoe_utils.c @@ -171,7 +171,7 @@ int check_symbolic_name_for_interface(const char *symbolic_name, int rc = -EINVAL; char *symb; - if (!strlen(ifname)) + if (!ifname) return rc; symb = get_ifname_from_symbolic_name(symbolic_name); diff --git a/fcoeadm.c b/fcoeadm.c index 64135af..dd05661 100644 --- a/fcoeadm.c +++ b/fcoeadm.c @@ -40,7 +40,6 @@ static struct option fcoeadm_opts[] = { {0, 0, 0, 0} }; -struct opt_info _opt_info, *opt_info = &_opt_info; char progname[20]; static void fcoeadm_help(void) @@ -214,9 +213,10 @@ static enum fcoe_err fcoeadm_action(enum clif_action cmd, char *ifname) */ int main(int argc, char *argv[]) { - int opt; enum clif_action cmd = CLIF_NONE; enum fcoe_err rc = NOERR; + int opt, stat_interval; + char *ifname = NULL; /* * This has to be first because the error print macro @@ -235,8 +235,6 @@ int main(int argc, char *argv[]) if (rc) goto err; - memset(opt_info, 0, sizeof(*opt_info)); - opt = getopt_long(argc, argv, optstring, fcoeadm_opts, NULL); if (opt != -1) { switch (opt) { @@ -257,19 +255,18 @@ int main(int argc, char *argv[]) break; } - strncpy(opt_info->ifname, optarg, - sizeof(opt_info->ifname)); + ifname = optarg; if (opt == 'c') { - rc = fcoe_validate_interface(opt_info->ifname); + rc = fcoe_validate_interface(ifname); if (!rc && - !fcoe_validate_fcoe_conn(opt_info->ifname)) + !fcoe_validate_fcoe_conn(ifname)) rc = EFCOECONN; } else - rc = fcoe_validate_fcoe_conn(opt_info->ifname); + rc = fcoe_validate_fcoe_conn(ifname); if (!rc) - rc = fcoeadm_action(cmd, opt_info->ifname); + rc = fcoeadm_action(cmd, ifname); break; case 'i': @@ -283,14 +280,12 @@ int main(int argc, char *argv[]) * treat it as the interface name. */ if (optind != argc) { - strncpy(opt_info->ifname, argv[optind], - sizeof(opt_info->ifname)); - - rc = fcoe_validate_fcoe_conn(opt_info->ifname); + ifname = argv[optind]; + rc = fcoe_validate_fcoe_conn(ifname); } if (!rc) - rc = display_adapter_info(opt_info); + rc = display_adapter_info(ifname); break; @@ -305,16 +300,12 @@ int main(int argc, char *argv[]) * treat it as the interface name. */ if (optind != argc) { - strncpy(opt_info->ifname, argv[optind], - sizeof(opt_info->ifname)); - - rc = fcoe_validate_fcoe_conn(opt_info->ifname); + ifname = argv[optind]; + rc = fcoe_validate_fcoe_conn(ifname); } - if (!rc) { - opt_info->t_flag = 1; - rc = display_target_info(opt_info); - } + if (!rc) + rc = display_target_info(ifname, DISP_TARG); break; @@ -329,16 +320,12 @@ int main(int argc, char *argv[]) * treat it as the interface name. */ if (optind != argc) { - strncpy(opt_info->ifname, argv[optind], - sizeof(opt_info->ifname)); - - rc = fcoe_validate_fcoe_conn(opt_info->ifname); + ifname = argv[optind]; + rc = fcoe_validate_fcoe_conn(ifname); } - if (!rc) { - opt_info->l_flag = 1; - rc = display_target_info(opt_info); - } + if (!rc) + rc = display_target_info(ifname, DISP_LUN); break; @@ -349,26 +336,19 @@ int main(int argc, char *argv[]) } if (optind != argc) { - strncpy(opt_info->ifname, argv[optind], - sizeof(opt_info->ifname)); - - rc = fcoe_validate_fcoe_conn(opt_info->ifname); + ifname = argv[optind]; + rc = fcoe_validate_fcoe_conn(ifname); } if (!rc && ++optind != argc) { - opt_info->n_interval = atoi(argv[optind]); - if (opt_info->n_interval <= 0) + stat_interval = atoi(argv[optind]); + if (stat_interval <= 0) rc = EINVALARG; - else - opt_info->n_flag = 1; } else if (!rc && optind == argc) - opt_info->n_interval = DEFAULT_STATS_INTERVAL; - - if (!rc) { - opt_info->s_flag = 1; - rc = display_port_stats(opt_info); - } + stat_interval = DEFAULT_STATS_INTERVAL; + if (!rc) + rc = display_port_stats(ifname, stat_interval); break; case 'v': @@ -400,12 +380,12 @@ err: switch (rc) { case EFCOECONN: FCOE_LOG_ERR("Connection already created on " - "interface %s\n", opt_info->ifname); + "interface %s\n", ifname); break; case ENOFCOECONN: FCOE_LOG_ERR("No connection created on " - "interface %s\n", opt_info->ifname); + "interface %s\n", ifname); break; case EINVALARG: @@ -428,8 +408,7 @@ err: break; case ENOETHDEV: - FCOE_LOG_ERR("Invalid interface name %s\n", - opt_info->ifname); + FCOE_LOG_ERR("Invalid interface name %s\n", ifname); break; case ENOSYSFS: diff --git a/fcoeadm.h b/fcoeadm.h index c721818..b3bc261 100644 --- a/fcoeadm.h +++ b/fcoeadm.h @@ -48,23 +48,17 @@ #include "hbaapi.h" #include "fcoe_utils.h" -struct opt_info { - char ifname[IFNAMSIZ]; - char a_flag; - char t_flag; - char l_flag; - char l_fcid_present; - HBA_UINT32 l_fcid; - char l_lun_id_present; - u_int32_t l_lun_id; - char s_flag; - char n_flag; - #define DEFAULT_STATS_INTERVAL 1 - int n_interval; /* seconds */ +#define DEFAULT_STATS_INTERVAL 1 + +enum disp_style { + DISP_LUN = 0, + DISP_TARG, }; -enum fcoe_err display_adapter_info(struct opt_info *opt_info); -enum fcoe_err display_target_info(struct opt_info *opt_info); -enum fcoe_err display_port_stats(struct opt_info *opt_info); +enum fcoe_err display_adapter_info(const char *ifname); +enum fcoe_err display_target_info(const char *ifname, + enum disp_style style); +enum fcoe_err display_port_stats(const char *ifname, + int stat_interval); #endif /* _FCOEADM_H_ */ diff --git a/fcoeadm_display.c b/fcoeadm_display.c index 203a698..d485f3b 100644 --- a/fcoeadm_display.c +++ b/fcoeadm_display.c @@ -905,7 +905,8 @@ scan_device_map(HBA_HANDLE hba_handle, HBA_ADAPTERATTRIBUTES *hba_info, HBA_PORTATTRIBUTES *lp_info, HBA_PORTATTRIBUTES *rp_info, - struct opt_info *opt_info) + const char *ifname, + enum disp_style style) { HBA_STATUS status; HBA_FCP_TARGET_MAPPING *map = NULL; @@ -929,11 +930,6 @@ scan_device_map(HBA_HANDLE hba_handle, if (ep->FcpId.FcId != rp_info->PortFcId) continue; - if (opt_info->l_flag && - opt_info->l_fcid_present && - opt_info->l_lun_id_present && - ep->ScsiId.ScsiOSLun != opt_info->l_lun_id) - continue; dev = ep->ScsiId.OSDeviceName; if (strstr(dev, "/dev/") == dev) dev += 5; @@ -961,15 +957,19 @@ scan_device_map(HBA_HANDLE hba_handle, #endif if (status != HBA_STATUS_OK) continue; - if (opt_info->t_flag) { + switch (style) { + case DISP_TARG: if (!print_header) { show_short_lun_info_header(); print_header = 1; } show_short_lun_info(ep, inqbuf, &rcap_resp); - } else if (opt_info->l_flag) + break; + case DISP_LUN: show_full_lun_info(hba_handle, hba_info, lp_info, - rp_info, ep, inqbuf, &rcap_resp); + rp_info, ep, inqbuf, &rcap_resp); + break; + } #ifdef TEST_REPORT_LUNS if (i == 0) { /* only issue report luns to the first LUN */ @@ -983,25 +983,24 @@ scan_device_map(HBA_HANDLE hba_handle, } /* Newline at the end of the short lun report */ - if (opt_info->t_flag) + if (style == DISP_TARG) printf("\n"); free(map); } -static void -show_port_stats_header(struct opt_info *opt_info) +static void show_port_stats_header(const char *ifname, int interval) { printf("\n"); printf("%-7s interval: %-2d Err Inv " - "IvTx Link Cntl Input Input Output Output\n", - opt_info->ifname, opt_info->n_interval); + "IvTx Link Cntl Input Input Output Output\n", + ifname, interval); printf("Seconds TxFrames TxBytes RxFrames RxBytes " - "Frms CRC Byte Fail Reqs Requests MBytes " - "Requests MBytes\n"); + "Frms CRC Byte Fail Reqs Requests MBytes " + "Requests MBytes\n"); printf("------- --------- ------------ --------- -------------- " - "---- ---- ---- ---- ---- --------- --------- " - "--------- ---------\n"); + "---- ---- ---- ---- ---- --------- --------- " + "--------- ---------\n"); } static void @@ -1135,7 +1134,7 @@ static int get_index_for_ifname(struct hba_name_table *hba_table, return -EINVAL; } -enum fcoe_err display_port_stats(struct opt_info *opt_info) +enum fcoe_err display_port_stats(const char *ifname, int interval) { HBA_STATUS retval; HBA_HANDLE hba_handle; @@ -1152,8 +1151,7 @@ enum fcoe_err display_port_stats(struct opt_info *opt_info) num_hbas = hba_table_init(hba_table); - i = get_index_for_ifname(hba_table, num_hbas, - opt_info->ifname); + i = get_index_for_ifname(hba_table, num_hbas, ifname); /* * Return error code if a valid index wasn't returned. @@ -1183,7 +1181,7 @@ enum fcoe_err display_port_stats(struct opt_info *opt_info) if (retval == HBA_STATUS_ERROR_NOT_SUPPORTED) { fprintf(stderr, "Port Statistics not supported by %s\n", - opt_info->ifname); + ifname); break; } @@ -1203,16 +1201,16 @@ enum fcoe_err display_port_stats(struct opt_info *opt_info) if (retval == HBA_STATUS_ERROR_NOT_SUPPORTED) { fprintf(stderr, "Port FC4 Statistics not supported by %s\n", - opt_info->ifname); + ifname); break; } if (!(i % 52)) - show_port_stats_header(opt_info); + show_port_stats_header(ifname, interval); show_port_stats_in_row(start_time, &port_stats, &port_fc4stats); i++; /* wait for the requested time interval in seconds */ - secs_left = opt_info->n_interval; + secs_left = interval; do { secs_left = sleep(secs_left); } while (secs_left); @@ -1223,7 +1221,7 @@ enum fcoe_err display_port_stats(struct opt_info *opt_info) return rc; } -enum fcoe_err display_adapter_info(struct opt_info *opt_info) +enum fcoe_err display_adapter_info(const char *ifname) { struct hba_name_table hba_table[MAX_HBA_COUNT]; enum fcoe_err rc = NOERR; @@ -1244,10 +1242,9 @@ enum fcoe_err display_adapter_info(struct opt_info *opt_info) hba_table[i].displayed) continue; - if (strlen(opt_info->ifname) && - check_symbolic_name_for_interface( + if (ifname && check_symbolic_name_for_interface( hba_table[i].port_attrs.PortSymbolicName, - opt_info->ifname)) { + ifname)) { /* * Overloading 'displayed' to indicate * that the HBA/Port should be skipped. @@ -1265,10 +1262,9 @@ enum fcoe_err display_adapter_info(struct opt_info *opt_info) * Loop through HBAs again to print sub-ports. */ for (j = 0; j < num_hbas ; j++) { - if (strlen(opt_info->ifname) && - check_symbolic_name_for_interface( + if (ifname && check_symbolic_name_for_interface( hba_table[j].port_attrs.PortSymbolicName, - opt_info->ifname)) { + ifname)) { /* * Overloading 'displayed' to indicate * that the HBA/Port should be skipped. @@ -1294,7 +1290,8 @@ enum fcoe_err display_adapter_info(struct opt_info *opt_info) return rc; } -enum fcoe_err display_target_info(struct opt_info *opt_info) +enum fcoe_err display_target_info(const char *ifname, + enum disp_style style) { HBA_STATUS retval; HBA_PORTATTRIBUTES rport_attrs; @@ -1317,10 +1314,9 @@ enum fcoe_err display_target_info(struct opt_info *opt_info) hba_table[i].displayed) continue; - if (strlen(opt_info->ifname) && - check_symbolic_name_for_interface( + if (ifname && check_symbolic_name_for_interface( hba_table[i].port_attrs.PortSymbolicName, - opt_info->ifname)) { + ifname)) { /* * Overloading 'displayed' to indicate * that the HBA/Port should be skipped. @@ -1349,16 +1345,6 @@ enum fcoe_err display_target_info(struct opt_info *opt_info) } /* - * If -l option and fcid are specified in the - * command, filter out the targets do not have - * port ID equals to fcid. - */ - if (opt_info->l_flag && - opt_info->l_fcid_present && - rport_attrs.PortFcId != opt_info->l_fcid) - continue; - - /* * Skip any targets that are not FCP targets */ if (is_fcp_target(&rport_attrs)) @@ -1377,7 +1363,7 @@ enum fcoe_err display_target_info(struct opt_info *opt_info) scan_device_map(hba_table[i].hba_handle, &hba_table[i].hba_attrs, &hba_table[i].port_attrs, - &rport_attrs, opt_info); + &rport_attrs, ifname, style); } } _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
