List comments

GET /api/patches/437/comments/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

[
    {
        "id": 899,
        "web_url": "https://patches.dpdk.org/comment/899/",
        "msgid": "<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": "https://patches.dpdk.org/api/people/32/",
            "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": {
            "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>",
            "Content-Disposition": "inline",
            "X-Mailman-Version": "2.1.15",
            "X-Spam-Status": "No",
            "Precedence": "list",
            "X-Original-To": "patchwork@dpdk.org",
            "List-Post": "<mailto:dev@dpdk.org>",
            "MIME-Version": "1.0",
            "List-Id": "patches and discussions about DPDK <dev.dpdk.org>",
            "X-BeenThere": "dev@dpdk.org",
            "References": "<1411375081-27986-1-git-send-email-david.marchand@6wind.com>\n\t<1411375081-27986-7-git-send-email-david.marchand@6wind.com>",
            "Subject": "Re: [dpdk-dev] [PATCH 6/7] eal: rework long options parsing",
            "User-Agent": "Mutt/1.5.23 (2014-03-12)",
            "Content-Type": "text/plain; charset=us-ascii",
            "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"
            ],
            "X-Spam-Score": "-2.9 (--)",
            "List-Archive": "<http://dpdk.org/ml/archives/dev/>",
            "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>",
            "Message-ID": "<20140922124208.GC25406@hmsreliant.think-freely.org>",
            "Date": "Mon, 22 Sep 2014 08:42:08 -0400",
            "Sender": "\"dev\" <dev-bounces@dpdk.org>",
            "To": "David Marchand <david.marchand@6wind.com>",
            "From": "Neil Horman <nhorman@tuxdriver.com>",
            "In-Reply-To": "<1411375081-27986-7-git-send-email-david.marchand@6wind.com>",
            "Cc": "dev@dpdk.org",
            "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
            "Errors-To": "dev-bounces@dpdk.org",
            "Return-Path": "<dev-bounces@dpdk.org>"
        }
    }
]