Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/49585/?format=api
{ "id": 49585, "url": "http://patches.dpdk.org/api/patches/49585/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/4e041e83fb00d8d818682997f795928c36b3283a.1547127516.git.anatoly.burakov@intel.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<4e041e83fb00d8d818682997f795928c36b3283a.1547127516.git.anatoly.burakov@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/4e041e83fb00d8d818682997f795928c36b3283a.1547127516.git.anatoly.burakov@intel.com", "date": "2019-01-10T13:38:59", "name": "eal: fix strdup usages in internal config", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "f18c40ecb47ee559b4b55667cf88177e993bb929", "submitter": { "id": 4, "url": "http://patches.dpdk.org/api/people/4/?format=api", "name": "Anatoly Burakov", "email": "anatoly.burakov@intel.com" }, "delegate": { "id": 1, "url": "http://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/4e041e83fb00d8d818682997f795928c36b3283a.1547127516.git.anatoly.burakov@intel.com/mbox/", "series": [ { "id": 3093, "url": "http://patches.dpdk.org/api/series/3093/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=3093", "date": "2019-01-10T13:38:59", "name": "eal: fix strdup usages in internal config", "version": 1, "mbox": "http://patches.dpdk.org/series/3093/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/49585/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/49585/checks/", "tags": {}, "related": [], "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 [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id BDF7C1B6CD;\n\tThu, 10 Jan 2019 14:39:06 +0100 (CET)", "from mga06.intel.com (mga06.intel.com [134.134.136.31])\n\tby dpdk.org (Postfix) with ESMTP id EC58B1B6CC\n\tfor <dev@dpdk.org>; Thu, 10 Jan 2019 14:39:04 +0100 (CET)", "from fmsmga005.fm.intel.com ([10.253.24.32])\n\tby orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t10 Jan 2019 05:39:03 -0800", "from irvmail001.ir.intel.com ([163.33.26.43])\n\tby fmsmga005.fm.intel.com with ESMTP; 10 Jan 2019 05:39:02 -0800", "from sivswdev05.ir.intel.com (sivswdev05.ir.intel.com\n\t[10.243.17.64])\n\tby irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id\n\tx0ADd1c7017739; Thu, 10 Jan 2019 13:39:01 GMT", "from sivswdev05.ir.intel.com (localhost [127.0.0.1])\n\tby sivswdev05.ir.intel.com with ESMTP id x0ADd16c006224;\n\tThu, 10 Jan 2019 13:39:01 GMT", "(from aburakov@localhost)\n\tby sivswdev05.ir.intel.com with LOCAL id x0ADd02n006182;\n\tThu, 10 Jan 2019 13:39:00 GMT" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.56,461,1539673200\"; d=\"scan'208\";a=\"310727684\"", "From": "Anatoly Burakov <anatoly.burakov@intel.com>", "To": "dev@dpdk.org", "Cc": "Bruce Richardson <bruce.richardson@intel.com>, thomas@monjalon.net,\n\tferruh.yigit@intel.com, andy01011501@163.com", "Date": "Thu, 10 Jan 2019 13:38:59 +0000", "Message-Id": "<4e041e83fb00d8d818682997f795928c36b3283a.1547127516.git.anatoly.burakov@intel.com>", "X-Mailer": "git-send-email 1.7.0.7", "Subject": "[dpdk-dev] [PATCH] eal: fix strdup usages in internal config", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Currently, we use strdup in a few places to store command-line\nparameter values for certain internal config values. There are\nseveral issues with that.\n\nFirst of all, they're never freed, so memory ends up leaking\neither after EAL exit, or when these command-line options are\nsupplied multiple times.\n\nSecond of all, they're defined as `const char *`, so they\n*cannot* be freed even if we wanted to.\n\nFinally, strdup may return NULL, which will be stored in the\nconfig. For most fields, NULL is a valid value, but for the\ndefault prefix, the value is always expected to be valid.\n\nTo fix all of this, three things are done. First, we change\nthe definitions of these values to `char *` as opposed to\n`const char *`. This does not break the ABI, and previous\ncode assumes constness (which is more restrictive), so it's\nsafe to do so.\n\nThen, fix all usages of strdup to check return value, and add\na cleanup function that will free the memory occupied by\nthese strings, as well as freeing them before assigning a new\nvalue to prevent leaks when parameter is specified multiple\ntimes.\n\nAnd finally, add an internal API to query hugefile prefix, so\nthat, absent of a valid value, a default value will be\nreturned, and also fix up all usages of hugefile prefix to\nuse this API instead of accessing hugefile prefix directly.\n\nBugzilla ID: 108\n\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n---\n lib/librte_eal/bsdapp/eal/eal.c | 19 +++++++--\n lib/librte_eal/common/eal_common_options.c | 25 +++++++++++-\n lib/librte_eal/common/eal_filesystem.h | 6 ++-\n lib/librte_eal/common/eal_internal_cfg.h | 6 +--\n lib/librte_eal/common/eal_options.h | 1 +\n lib/librte_eal/linuxapp/eal/eal.c | 46 ++++++++++++++++++----\n lib/librte_eal/linuxapp/eal/eal_memory.c | 2 +-\n 7 files changed, 87 insertions(+), 18 deletions(-)", "diff": "diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c\nindex c8e0da097..1ba9bd7cf 100644\n--- a/lib/librte_eal/bsdapp/eal/eal.c\n+++ b/lib/librte_eal/bsdapp/eal/eal.c\n@@ -117,7 +117,7 @@ eal_create_runtime_dir(void)\n \n \t/* create prefix-specific subdirectory under DPDK runtime dir */\n \tret = snprintf(runtime_dir, sizeof(runtime_dir), \"%s/%s\",\n-\t\t\ttmp, internal_config.hugefile_prefix);\n+\t\t\ttmp, eal_get_hugefile_prefix());\n \tif (ret < 0 || ret == sizeof(runtime_dir)) {\n \t\tRTE_LOG(ERR, EAL, \"Error creating prefix-specific runtime path name\\n\");\n \t\treturn -1;\n@@ -535,9 +535,21 @@ eal_parse_args(int argc, char **argv)\n \n \t\tswitch (opt) {\n \t\tcase OPT_MBUF_POOL_OPS_NAME_NUM:\n-\t\t\tinternal_config.user_mbuf_pool_ops_name =\n-\t\t\t strdup(optarg);\n+\t\t{\n+\t\t\tchar *ops_name = strdup(optarg);\n+\t\t\tif (ops_name == NULL)\n+\t\t\t\tRTE_LOG(ERR, EAL, \"Could not store mbuf pool ops name\\n\");\n+\t\t\telse {\n+\t\t\t\t/* free old ops name */\n+\t\t\t\tif (internal_config.user_mbuf_pool_ops_name !=\n+\t\t\t\t\t\tNULL)\n+\t\t\t\t\tfree(internal_config.user_mbuf_pool_ops_name);\n+\n+\t\t\t\tinternal_config.user_mbuf_pool_ops_name =\n+\t\t\t\t\t\tops_name;\n+\t\t\t}\n \t\t\tbreak;\n+\t\t}\n \t\tcase 'h':\n \t\t\teal_usage(prgname);\n \t\t\texit(EXIT_SUCCESS);\n@@ -923,6 +935,7 @@ rte_eal_cleanup(void)\n {\n \trte_service_finalize();\n \trte_mp_channel_cleanup();\n+\teal_cleanup_config(&internal_config);\n \treturn 0;\n }\n \ndiff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c\nindex 6e3a83b98..a2d862b5f 100644\n--- a/lib/librte_eal/common/eal_common_options.c\n+++ b/lib/librte_eal/common/eal_common_options.c\n@@ -169,6 +169,14 @@ eal_option_device_parse(void)\n \treturn ret;\n }\n \n+const char *\n+eal_get_hugefile_prefix(void)\n+{\n+\tif (internal_config.hugefile_prefix != NULL)\n+\t\treturn internal_config.hugefile_prefix;\n+\treturn HUGEFILE_PREFIX_DEFAULT;\n+}\n+\n void\n eal_reset_internal_config(struct internal_config *internal_cfg)\n {\n@@ -177,7 +185,7 @@ eal_reset_internal_config(struct internal_config *internal_cfg)\n \tinternal_cfg->memory = 0;\n \tinternal_cfg->force_nrank = 0;\n \tinternal_cfg->force_nchannel = 0;\n-\tinternal_cfg->hugefile_prefix = HUGEFILE_PREFIX_DEFAULT;\n+\tinternal_cfg->hugefile_prefix = NULL;\n \tinternal_cfg->hugepage_dir = NULL;\n \tinternal_cfg->force_sockets = 0;\n \t/* zero out the NUMA config */\n@@ -1347,6 +1355,19 @@ eal_auto_detect_cores(struct rte_config *cfg)\n \tcfg->lcore_count -= removed;\n }\n \n+int\n+eal_cleanup_config(struct internal_config *internal_cfg)\n+{\n+\tif (internal_cfg->hugefile_prefix != NULL)\n+\t\tfree(internal_cfg->hugefile_prefix);\n+\tif (internal_cfg->hugepage_dir != NULL)\n+\t\tfree(internal_cfg->hugepage_dir);\n+\tif (internal_cfg->user_mbuf_pool_ops_name != NULL)\n+\t\tfree(internal_cfg->user_mbuf_pool_ops_name);\n+\n+\treturn 0;\n+}\n+\n int\n eal_adjust_config(struct internal_config *internal_cfg)\n {\n@@ -1387,7 +1408,7 @@ eal_check_common_options(struct internal_config *internal_cfg)\n \t\tRTE_LOG(ERR, EAL, \"Invalid process type specified\\n\");\n \t\treturn -1;\n \t}\n-\tif (index(internal_cfg->hugefile_prefix, '%') != NULL) {\n+\tif (index(eal_get_hugefile_prefix(), '%') != NULL) {\n \t\tRTE_LOG(ERR, EAL, \"Invalid char, '%%', in --\"OPT_FILE_PREFIX\" \"\n \t\t\t\"option\\n\");\n \t\treturn -1;\ndiff --git a/lib/librte_eal/common/eal_filesystem.h b/lib/librte_eal/common/eal_filesystem.h\nindex 64a028db7..89a3added 100644\n--- a/lib/librte_eal/common/eal_filesystem.h\n+++ b/lib/librte_eal/common/eal_filesystem.h\n@@ -28,6 +28,10 @@ eal_create_runtime_dir(void);\n int\n eal_clean_runtime_dir(void);\n \n+/** Function to return hugefile prefix that's currently set up */\n+const char *\n+eal_get_hugefile_prefix(void);\n+\n #define RUNTIME_CONFIG_FNAME \"config\"\n static inline const char *\n eal_runtime_config_path(void)\n@@ -89,7 +93,7 @@ static inline const char *\n eal_get_hugefile_path(char *buffer, size_t buflen, const char *hugedir, int f_id)\n {\n \tsnprintf(buffer, buflen, HUGEFILE_FMT, hugedir,\n-\t\t\tinternal_config.hugefile_prefix, f_id);\n+\t\t\teal_get_hugefile_prefix(), f_id);\n \tbuffer[buflen - 1] = '\\0';\n \treturn buffer;\n }\ndiff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h\nindex 98e314fef..60eaead8f 100644\n--- a/lib/librte_eal/common/eal_internal_cfg.h\n+++ b/lib/librte_eal/common/eal_internal_cfg.h\n@@ -66,9 +66,9 @@ struct internal_config {\n \tvolatile int syslog_facility;\t /**< facility passed to openlog() */\n \t/** default interrupt mode for VFIO */\n \tvolatile enum rte_intr_mode vfio_intr_mode;\n-\tconst char *hugefile_prefix; /**< the base filename of hugetlbfs files */\n-\tconst char *hugepage_dir; /**< specific hugetlbfs directory to use */\n-\tconst char *user_mbuf_pool_ops_name;\n+\tchar *hugefile_prefix; /**< the base filename of hugetlbfs files */\n+\tchar *hugepage_dir; /**< specific hugetlbfs directory to use */\n+\tchar *user_mbuf_pool_ops_name;\n \t\t\t/**< user defined mbuf pool ops name */\n \tunsigned num_hugepage_sizes; /**< how many sizes on this system */\n \tstruct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];\ndiff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h\nindex 1480c5d77..58ee9ae33 100644\n--- a/lib/librte_eal/common/eal_options.h\n+++ b/lib/librte_eal/common/eal_options.h\n@@ -77,6 +77,7 @@ int eal_parse_common_option(int opt, const char *argv,\n \t\t\t struct internal_config *conf);\n int eal_option_device_parse(void);\n int eal_adjust_config(struct internal_config *internal_cfg);\n+int eal_cleanup_config(struct internal_config *internal_cfg);\n int eal_check_common_options(struct internal_config *internal_cfg);\n void eal_common_usage(void);\n enum rte_proc_type_t eal_proc_type_detect(void);\ndiff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c\nindex 2d8d470b8..a386829f3 100644\n--- a/lib/librte_eal/linuxapp/eal/eal.c\n+++ b/lib/librte_eal/linuxapp/eal/eal.c\n@@ -125,7 +125,7 @@ eal_create_runtime_dir(void)\n \n \t/* create prefix-specific subdirectory under DPDK runtime dir */\n \tret = snprintf(runtime_dir, sizeof(runtime_dir), \"%s/%s\",\n-\t\t\ttmp, internal_config.hugefile_prefix);\n+\t\t\ttmp, eal_get_hugefile_prefix());\n \tif (ret < 0 || ret == sizeof(runtime_dir)) {\n \t\tRTE_LOG(ERR, EAL, \"Error creating prefix-specific runtime path name\\n\");\n \t\treturn -1;\n@@ -727,13 +727,31 @@ eal_parse_args(int argc, char **argv)\n \t\t\texit(EXIT_SUCCESS);\n \n \t\tcase OPT_HUGE_DIR_NUM:\n-\t\t\tinternal_config.hugepage_dir = strdup(optarg);\n+\t\t{\n+\t\t\tchar *hdir = strdup(optarg);\n+\t\t\tif (hdir == NULL)\n+\t\t\t\tRTE_LOG(ERR, EAL, \"Could not store hugepage directory\\n\");\n+\t\t\telse {\n+\t\t\t\t/* free old hugepage dir */\n+\t\t\t\tif (internal_config.hugepage_dir != NULL)\n+\t\t\t\t\tfree(internal_config.hugepage_dir);\n+\t\t\t\tinternal_config.hugepage_dir = hdir;\n+\t\t\t}\n \t\t\tbreak;\n-\n+\t\t}\n \t\tcase OPT_FILE_PREFIX_NUM:\n-\t\t\tinternal_config.hugefile_prefix = strdup(optarg);\n+\t\t{\n+\t\t\tchar *prefix = strdup(optarg);\n+\t\t\tif (prefix == NULL)\n+\t\t\t\tRTE_LOG(ERR, EAL, \"Could not store file prefix\\n\");\n+\t\t\telse {\n+\t\t\t\t/* free old prefix */\n+\t\t\t\tif (internal_config.hugefile_prefix != NULL)\n+\t\t\t\t\tfree(internal_config.hugefile_prefix);\n+\t\t\t\tinternal_config.hugefile_prefix = prefix;\n+\t\t\t}\n \t\t\tbreak;\n-\n+\t\t}\n \t\tcase OPT_SOCKET_MEM_NUM:\n \t\t\tif (eal_parse_socket_arg(optarg,\n \t\t\t\t\tinternal_config.socket_mem) < 0) {\n@@ -783,10 +801,21 @@ eal_parse_args(int argc, char **argv)\n \t\t\tbreak;\n \n \t\tcase OPT_MBUF_POOL_OPS_NAME_NUM:\n-\t\t\tinternal_config.user_mbuf_pool_ops_name =\n-\t\t\t strdup(optarg);\n-\t\t\tbreak;\n+\t\t{\n+\t\t\tchar *ops_name = strdup(optarg);\n+\t\t\tif (ops_name == NULL)\n+\t\t\t\tRTE_LOG(ERR, EAL, \"Could not store mbuf pool ops name\\n\");\n+\t\t\telse {\n+\t\t\t\t/* free old ops name */\n+\t\t\t\tif (internal_config.user_mbuf_pool_ops_name !=\n+\t\t\t\t\t\tNULL)\n+\t\t\t\t\tfree(internal_config.user_mbuf_pool_ops_name);\n \n+\t\t\t\tinternal_config.user_mbuf_pool_ops_name =\n+\t\t\t\t\t\tops_name;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\t}\n \t\tcase OPT_MATCH_ALLOCATIONS_NUM:\n \t\t\tinternal_config.match_allocations = 1;\n \t\t\tbreak;\n@@ -1238,6 +1267,7 @@ rte_eal_cleanup(void)\n \t\trte_memseg_walk(mark_freeable, NULL);\n \trte_service_finalize();\n \trte_mp_channel_cleanup();\n+\teal_cleanup_config(&internal_config);\n \treturn 0;\n }\n \ndiff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c\nindex 7d922a965..1b96b576e 100644\n--- a/lib/librte_eal/linuxapp/eal/eal_memory.c\n+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c\n@@ -438,7 +438,7 @@ find_numasocket(struct hugepage_file *hugepg_tbl, struct hugepage_info *hpi)\n \t}\n \n \tsnprintf(hugedir_str, sizeof(hugedir_str),\n-\t\t\t\"%s/%s\", hpi->hugedir, internal_config.hugefile_prefix);\n+\t\t\t\"%s/%s\", hpi->hugedir, eal_get_hugefile_prefix());\n \n \t/* parse numa map */\n \twhile (fgets(buf, sizeof(buf), f) != NULL) {\n", "prefixes": [] }