Patch Comment List
List patch comments
GET /api/patches/437/comments/?format=api
[ { "id": 899, "web_url": "http://patches.dpdk.org/comment/899/", "msgid": "<20140922124208.GC25406@hmsreliant.think-freely.org>", "list_archive_url": "https://inbox.dpdk.org/dev/20140922124208.GC25406@hmsreliant.think-freely.org", "date": "2014-09-22T12:42:08", "subject": "Re: [dpdk-dev] [PATCH 6/7] eal: rework long options parsing", "submitter": { "id": 32, "url": "http://patches.dpdk.org/api/people/32/?format=api", "name": "Neil Horman", "email": "nhorman@tuxdriver.com" }, "content": "On Mon, Sep 22, 2014 at 10:38:00AM +0200, David Marchand wrote:\n> Identify all options through the getopt_long return value.\n> This way, we only need a big switch/case.\n> \n> Indentation is broken to ease commit review (fixed in next commit).\n> \n> Suggested-by: Neil Horman <nhorman@tuxdriver.com>\n> Signed-off-by: David Marchand <david.marchand@6wind.com>\n> ---\n> lib/librte_eal/bsdapp/eal/eal.c | 21 +++-----\n> lib/librte_eal/common/eal_common_options.c | 77 ++++++++++++++++-----------\n> lib/librte_eal/common/include/eal_options.h | 28 +++++++++-\n> lib/librte_eal/linuxapp/eal/eal.c | 44 ++++++++-------\n> 4 files changed, 105 insertions(+), 65 deletions(-)\n> \n> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c\n> index 58f00fd..c4b75a4 100644\n> --- a/lib/librte_eal/bsdapp/eal/eal.c\n> +++ b/lib/librte_eal/bsdapp/eal/eal.c\n> @@ -354,8 +354,7 @@ eal_parse_args(int argc, char **argv)\n> \t\tif (opt == '?')\n> \t\t\treturn -1;\n> \n> -\t\tret = eal_parse_common_option(opt, optarg, option_index,\n> -\t\t\t\t\t &internal_config);\n> +\t\tret = eal_parse_common_option(opt, optarg, &internal_config);\n> \t\t/* common parser is not happy */\n> \t\tif (ret < 0) {\n> \t\t\teal_usage(prgname);\n> @@ -371,21 +370,15 @@ eal_parse_args(int argc, char **argv)\n> \t\t}\n> \n> \t\tswitch (opt) {\n> -\t\t/* long options */\n> -\t\tcase 0:\n> -\t\t\t{\n> -\t\t\t\tRTE_LOG(ERR, EAL, \"Option %s is not supported \"\n> -\t\t\t\t\t\"on FreeBSD\\n\",\n> -\t\t\t\t\teal_long_options[option_index].name);\n> -\t\t\t\teal_usage(prgname);\n> -\t\t\t\treturn -1;\n> -\t\t\t}\n> -\t\t\tbreak;\n> -\n> \t\tdefault:\n> -\t\t\tif (isprint(opt)) {\n> +\t\t\tif (opt < OPT_LONG_MIN_NUM && isprint(opt)) {\n> \t\t\t\tRTE_LOG(ERR, EAL, \"Option %c is not supported \"\n> \t\t\t\t\t\"on FreeBSD\\n\", opt);\n> +\t\t\t} else if (opt >= OPT_LONG_MIN_NUM &&\n> +\t\t\t\t opt < OPT_LONG_MAX_NUM) {\n> +\t\t\t\tRTE_LOG(ERR, EAL, \"Option %s is not supported \"\n> +\t\t\t\t\t\"on FreeBSD\\n\",\n> +\t\t\t\t\teal_long_options[option_index].name);\n> \t\t\t} else {\n> \t\t\t\tRTE_LOG(ERR, EAL, \"Option %d is not supported \"\n> \t\t\t\t\t\"on FreeBSD\\n\", opt);\n> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c\n> index ead4300..4009f02 100644\n> --- a/lib/librte_eal/common/eal_common_options.c\n> +++ b/lib/librte_eal/common/eal_common_options.c\n> @@ -63,24 +63,24 @@ eal_short_options[] =\n> \n> const struct option\n> eal_long_options[] = {\n> -\t{OPT_HUGE_DIR, 1, 0, 0},\n> -\t{OPT_PROC_TYPE, 1, 0, 0},\n> -\t{OPT_NO_SHCONF, 0, 0, 0},\n> -\t{OPT_NO_HPET, 0, 0, 0},\n> -\t{OPT_VMWARE_TSC_MAP, 0, 0, 0},\n> -\t{OPT_NO_PCI, 0, 0, 0},\n> -\t{OPT_NO_HUGE, 0, 0, 0},\n> -\t{OPT_FILE_PREFIX, 1, 0, 0},\n> -\t{OPT_SOCKET_MEM, 1, 0, 0},\n> -\t{OPT_PCI_WHITELIST, 1, 0, 'w'},\n> -\t{OPT_PCI_BLACKLIST, 1, 0, 'b'},\n> -\t{OPT_VDEV, 1, 0, 0},\n> -\t{OPT_SYSLOG, 1, NULL, 0},\n> -\t{OPT_LOG_LEVEL, 1, NULL, 0},\n> -\t{OPT_BASE_VIRTADDR, 1, 0, 0},\n> -\t{OPT_XEN_DOM0, 0, 0, 0},\n> -\t{OPT_CREATE_UIO_DEV, 1, NULL, 0},\n> -\t{OPT_VFIO_INTR, 1, NULL, 0},\n> +\t{OPT_HUGE_DIR, 1, 0, OPT_HUGE_DIR_NUM},\n> +\t{OPT_PROC_TYPE, 1, 0, OPT_PROC_TYPE_NUM},\n> +\t{OPT_NO_SHCONF, 0, 0, OPT_NO_SHCONF_NUM},\n> +\t{OPT_NO_HPET, 0, 0, OPT_NO_HPET_NUM},\n> +\t{OPT_VMWARE_TSC_MAP, 0, 0, OPT_VMWARE_TSC_MAP_NUM},\n> +\t{OPT_NO_PCI, 0, 0, OPT_NO_PCI_NUM},\n> +\t{OPT_NO_HUGE, 0, 0, OPT_NO_HUGE_NUM},\n> +\t{OPT_FILE_PREFIX, 1, 0, OPT_FILE_PREFIX_NUM},\n> +\t{OPT_SOCKET_MEM, 1, 0, OPT_SOCKET_MEM_NUM},\n> +\t{OPT_PCI_WHITELIST, 1, 0, OPT_PCI_WHITELIST_NUM},\n> +\t{OPT_PCI_BLACKLIST, 1, 0, OPT_PCI_BLACKLIST_NUM},\n> +\t{OPT_VDEV, 1, 0, OPT_VDEV_NUM},\n> +\t{OPT_SYSLOG, 1, NULL, OPT_SYSLOG_NUM},\n> +\t{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},\n> +\t{OPT_BASE_VIRTADDR, 1, 0, OPT_BASE_VIRTADDR_NUM},\n> +\t{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},\n> +\t{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},\n> +\t{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},\n> \t{0, 0, 0, 0}\n> };\n> \n> @@ -238,7 +238,7 @@ eal_parse_proc_type(const char *arg)\n> }\n> \n> int\n> -eal_parse_common_option(int opt, const char *optarg, int longindex,\n> +eal_parse_common_option(int opt, const char *optarg,\n> \t\t\tstruct internal_config *conf)\n> {\n> \tswitch (opt) {\n> @@ -296,32 +296,46 @@ eal_parse_common_option(int opt, const char *optarg, int longindex,\n> \t\tbreak;\n> \n> \t/* long options */\n> -\tcase 0:\n> -\t\tif (!strcmp(eal_long_options[longindex].name, OPT_NO_HUGE)) {\n> +\tcase OPT_NO_HUGE_NUM:\n> \t\t\tconf->no_hugetlbfs = 1;\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_NO_PCI)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_NO_PCI_NUM:\n> \t\t\tconf->no_pci = 1;\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_NO_HPET)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_NO_HPET_NUM:\n> \t\t\tconf->no_hpet = 1;\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_VMWARE_TSC_MAP)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_VMWARE_TSC_MAP_NUM:\n> \t\t\tconf->vmware_tsc_map = 1;\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_NO_SHCONF)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_NO_SHCONF_NUM:\n> \t\t\tconf->no_shconf = 1;\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_PROC_TYPE)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_PROC_TYPE_NUM:\n> \t\t\tconf->process_type = eal_parse_proc_type(optarg);\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_VDEV)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_VDEV_NUM:\n> \t\t\tif (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,\n> \t\t\t\t\toptarg) < 0) {\n> \t\t\t\treturn -1;\n> \t\t\t}\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name, OPT_SYSLOG)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_SYSLOG_NUM:\n> \t\t\tif (eal_parse_syslog(optarg, conf) < 0) {\n> \t\t\t\tRTE_LOG(ERR, EAL, \"invalid parameters for --\"\n> \t\t\t\t\t\tOPT_SYSLOG \"\\n\");\n> \t\t\t\treturn -1;\n> \t\t\t}\n> -\t\t} else if (!strcmp(eal_long_options[longindex].name,\n> -\t\t\t\t OPT_LOG_LEVEL)) {\n> +\t\tbreak;\n> +\n> +\tcase OPT_LOG_LEVEL_NUM: {\n> \t\t\tuint32_t log;\n> \n> \t\t\tif (eal_parse_log_level(optarg, &log) < 0) {\n> @@ -331,9 +345,8 @@ eal_parse_common_option(int opt, const char *optarg, int longindex,\n> \t\t\t\treturn -1;\n> \t\t\t}\n> \t\t\tconf->log_level = log;\n> -\t\t}\n> -\n> \t\tbreak;\n> +\t\t}\n> \n> \t/* don't know what to do, leave this to caller */\n> \tdefault:\n> diff --git a/lib/librte_eal/common/include/eal_options.h b/lib/librte_eal/common/include/eal_options.h\n> index 4e6b90d..22819ec 100644\n> --- a/lib/librte_eal/common/include/eal_options.h\n> +++ b/lib/librte_eal/common/include/eal_options.h\n> @@ -30,29 +30,55 @@\n> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.\n> */\n> \n> +\n> +enum {\n> +\t/* long options mapped to a short option */\n> #define OPT_PCI_WHITELIST \"pci-whitelist\"\n> +\tOPT_PCI_WHITELIST_NUM = 'w',\n> #define OPT_PCI_BLACKLIST \"pci-blacklist\"\n> +\tOPT_PCI_BLACKLIST_NUM = 'b',\n> \n> +\t/* first long only option value must be >= 256, so that we won't\n> +\t * conflict with short options */\n> +\tOPT_LONG_MIN_NUM = 256,\n> #define OPT_HUGE_DIR \"huge-dir\"\n> +\tOPT_HUGE_DIR_NUM = OPT_LONG_MIN_NUM,\n> #define OPT_PROC_TYPE \"proc-type\"\n> +\tOPT_PROC_TYPE_NUM,\n> #define OPT_NO_SHCONF \"no-shconf\"\n> +\tOPT_NO_SHCONF_NUM,\n> #define OPT_NO_HPET \"no-hpet\"\n> +\tOPT_NO_HPET_NUM,\n> #define OPT_VMWARE_TSC_MAP \"vmware-tsc-map\"\n> +\tOPT_VMWARE_TSC_MAP_NUM,\n> #define OPT_NO_PCI \"no-pci\"\n> +\tOPT_NO_PCI_NUM,\n> #define OPT_NO_HUGE \"no-huge\"\n> +\tOPT_NO_HUGE_NUM,\n> #define OPT_FILE_PREFIX \"file-prefix\"\n> +\tOPT_FILE_PREFIX_NUM,\n> #define OPT_SOCKET_MEM \"socket-mem\"\n> +\tOPT_SOCKET_MEM_NUM,\n> #define OPT_VDEV \"vdev\"\n> +\tOPT_VDEV_NUM,\n> #define OPT_SYSLOG \"syslog\"\n> +\tOPT_SYSLOG_NUM,\n> #define OPT_LOG_LEVEL \"log-level\"\n> +\tOPT_LOG_LEVEL_NUM,\n> #define OPT_BASE_VIRTADDR \"base-virtaddr\"\n> +\tOPT_BASE_VIRTADDR_NUM,\n> #define OPT_XEN_DOM0 \"xen-dom0\"\n> +\tOPT_XEN_DOM0_NUM,\n> #define OPT_CREATE_UIO_DEV \"create-uio-dev\"\n> +\tOPT_CREATE_UIO_DEV_NUM,\n> #define OPT_VFIO_INTR \"vfio-intr\"\n> +\tOPT_VFIO_INTR_NUM,\n> +\tOPT_LONG_MAX_NUM\n> +};\n> \n> extern const char eal_short_options[];\n> extern const struct option eal_long_options[];\n> \n> -int eal_parse_common_option(int opt, const char *argv, int longindex,\n> +int eal_parse_common_option(int opt, const char *argv,\n> \t\t\t struct internal_config *conf);\n> void eal_common_usage(void);\n> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c\n> index d82debf..1d468dd 100644\n> --- a/lib/librte_eal/linuxapp/eal/eal.c\n> +++ b/lib/librte_eal/linuxapp/eal/eal.c\n> @@ -553,8 +553,7 @@ eal_parse_args(int argc, char **argv)\n> \t\tif (opt == '?')\n> \t\t\treturn -1;\n> \n> -\t\tret = eal_parse_common_option(opt, optarg, option_index,\n> -\t\t\t\t\t &internal_config);\n> +\t\tret = eal_parse_common_option(opt, optarg, &internal_config);\n> \t\t/* common parser is not happy */\n> \t\tif (ret < 0) {\n> \t\t\teal_usage(prgname);\n> @@ -584,8 +583,7 @@ eal_parse_args(int argc, char **argv)\n> \t\t\tbreak;\n> \n> \t\t/* long options */\n> -\t\tcase 0:\n> -\t\t\tif (!strcmp(eal_long_options[option_index].name, OPT_XEN_DOM0)) {\n> +\t\tcase OPT_XEN_DOM0_NUM:\n> \t\t#ifdef RTE_LIBRTE_XEN_DOM0\n> \t\t\t\tinternal_config.xen_dom0_support = 1;\n> \t\t#else\n> @@ -594,46 +592,56 @@ eal_parse_args(int argc, char **argv)\n> \t\t\t\t\t\" RTE_LIBRTE_XEN_DOM0=y\\n\");\n> \t\t\t\treturn -1;\n> \t\t#endif\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_HUGE_DIR)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_HUGE_DIR_NUM:\n> \t\t\t\tinternal_config.hugepage_dir = optarg;\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_FILE_PREFIX)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_FILE_PREFIX_NUM:\n> \t\t\t\tinternal_config.hugefile_prefix = optarg;\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_SOCKET_MEM)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_SOCKET_MEM_NUM:\n> \t\t\t\tif (eal_parse_socket_mem(optarg) < 0) {\n> \t\t\t\t\tRTE_LOG(ERR, EAL, \"invalid parameters for --\"\n> \t\t\t\t\t\t\tOPT_SOCKET_MEM \"\\n\");\n> \t\t\t\t\teal_usage(prgname);\n> \t\t\t\t\treturn -1;\n> \t\t\t\t}\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_BASE_VIRTADDR)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_BASE_VIRTADDR_NUM:\n> \t\t\t\tif (eal_parse_base_virtaddr(optarg) < 0) {\n> \t\t\t\t\tRTE_LOG(ERR, EAL, \"invalid parameter for --\"\n> \t\t\t\t\t\t\tOPT_BASE_VIRTADDR \"\\n\");\n> \t\t\t\t\teal_usage(prgname);\n> \t\t\t\t\treturn -1;\n> \t\t\t\t}\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_VFIO_INTR)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_VFIO_INTR_NUM:\n> \t\t\t\tif (eal_parse_vfio_intr(optarg) < 0) {\n> \t\t\t\t\tRTE_LOG(ERR, EAL, \"invalid parameters for --\"\n> \t\t\t\t\t\t\tOPT_VFIO_INTR \"\\n\");\n> \t\t\t\t\teal_usage(prgname);\n> \t\t\t\t\treturn -1;\n> \t\t\t\t}\n> -\t\t\t} else if (!strcmp(eal_long_options[option_index].name, OPT_CREATE_UIO_DEV)) {\n> +\t\t\tbreak;\n> +\n> +\t\tcase OPT_CREATE_UIO_DEV_NUM:\n> \t\t\t\tinternal_config.create_uio_dev = 1;\n> -\t\t\t} else {\n> -\t\t\t\tRTE_LOG(ERR, EAL, \"Option %s is not supported \"\n> -\t\t\t\t\t\"on Linux\\n\",\n> -\t\t\t\t\teal_long_options[option_index].name);\n> -\t\t\t\teal_usage(prgname);\n> -\t\t\t\treturn -1;\n> -\t\t\t}\n> \t\t\tbreak;\n> \n> \t\tdefault:\n> -\t\t\tif (isprint(opt)) {\n> +\t\t\tif (opt < OPT_LONG_MIN_NUM && isprint(opt)) {\n> \t\t\t\tRTE_LOG(ERR, EAL, \"Option %c is not supported \"\n> \t\t\t\t\t\"on Linux\\n\", opt);\n> +\t\t\t} else if (opt >= OPT_LONG_MIN_NUM &&\n> +\t\t\t\t opt < OPT_LONG_MAX_NUM) {\n> +\t\t\t\tRTE_LOG(ERR, EAL, \"Option %s is not supported \"\n> +\t\t\t\t\t\"on Linux\\n\",\n> +\t\t\t\t\teal_long_options[option_index].name);\n> \t\t\t} else {\n> \t\t\t\tRTE_LOG(ERR, EAL, \"Option %d is not supported \"\n> \t\t\t\t\t\"on Linux\\n\", opt);\n> -- \n> 1.7.10.4\n> \n> \nThanks! This looks much nicer\nNeil", "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 836B2B3AD;\n\tMon, 22 Sep 2014 14:36:16 +0200 (CEST)", "from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])\n\tby dpdk.org (Postfix) with ESMTP id 97CF5B3AC\n\tfor <dev@dpdk.org>; Mon, 22 Sep 2014 14:36:14 +0200 (CEST)", "from [2001:470:8:a08:18c5:c64e:4bf:67a] (helo=localhost)\n\tby smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)\n\t(envelope-from <nhorman@tuxdriver.com>)\n\tid 1XW2wT-00033u-Pq; Mon, 22 Sep 2014 08:42:16 -0400" ], "Date": "Mon, 22 Sep 2014 08:42:08 -0400", "From": "Neil Horman <nhorman@tuxdriver.com>", "To": "David Marchand <david.marchand@6wind.com>", "Message-ID": "<20140922124208.GC25406@hmsreliant.think-freely.org>", "References": "<1411375081-27986-1-git-send-email-david.marchand@6wind.com>\n\t<1411375081-27986-7-git-send-email-david.marchand@6wind.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=us-ascii", "Content-Disposition": "inline", "In-Reply-To": "<1411375081-27986-7-git-send-email-david.marchand@6wind.com>", "User-Agent": "Mutt/1.5.23 (2014-03-12)", "X-Spam-Score": "-2.9 (--)", "X-Spam-Status": "No", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH 6/7] eal: rework long options parsing", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "addressed": null } ]