As pointed out by Eric, files in the local directory with names of the
form 'nmemX' can break the recently added glob support by filtering
otherwise valid dimm names in a glob like 'nmem[X-Y]'. For robustness,
the glob argument would always need to be shell escaped, but at that
point it is less characters to simply rely on shell expansion of the
form nmem{X..Y}, if available.
Reported-by: Eric Blake <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
ndctl/builtin-dimm.c | 70 +++++++++-----------------------------------------
1 file changed, 13 insertions(+), 57 deletions(-)
diff --git a/ndctl/builtin-dimm.c b/ndctl/builtin-dimm.c
index 304bd83a33da..19036fd38b46 100644
--- a/ndctl/builtin-dimm.c
+++ b/ndctl/builtin-dimm.c
@@ -10,7 +10,6 @@
* FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for
* more details.
*/
-#include <glob.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
@@ -774,72 +773,35 @@ static int dimm_action(int argc, const char **argv,
struct ndctl_ctx *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context
*actx),
const struct option *options, const char *usage)
{
- int rc = 0, count, err = 0, glob_cnt = 0;
struct action_context actx = { NULL, NULL };
+ int i, rc = 0, count, err = 0;
const char * const u[] = {
usage,
NULL
};
- char *all[] = { "all " };
- glob_t glob_buf;
- size_t i;
+ unsigned long id;
argc = parse_options(argc, argv, options, u, 0);
if (argc == 0)
usage_with_options(u, options);
- for (i = 0; i < (size_t) argc; i++) {
- char *path;
-
+ for (i = 0; i < argc; i++) {
if (strcmp(argv[i], "all") == 0) {
argv[0] = "all";
argc = 1;
- glob_cnt = 0;
break;
}
- rc = asprintf(&path, "/sys/bus/nd/devices/%s", argv[i]);
- if (rc < 0) {
- fprintf(stderr, "failed to parse %s\n", argv[i]);
- usage_with_options(u, options);
- }
- rc = glob(path, glob_cnt++ ? GLOB_APPEND : 0, NULL, &glob_buf);
- switch (rc) {
- case GLOB_NOSPACE:
- case GLOB_ABORTED:
- fprintf(stderr, "failed to parse %s\n", argv[i]);
- usage_with_options(u, options);
- break;
- case GLOB_NOMATCH:
- case 0:
- break;
+ if (sscanf(argv[i], "nmem%lu", &id) != 1) {
+ fprintf(stderr, "'%s' is not a valid dimm name\n",
+ argv[i]);
+ err++;
}
- free(path);
- }
-
- if (!glob_cnt)
- glob_buf.gl_pathc = 0;
- count = 0;
- for (i = 0; i < glob_buf.gl_pathc; i++) {
- char *dimm_name = strrchr(glob_buf.gl_pathv[i], '/');
- unsigned long id;
-
- if (!dimm_name++)
- continue;
- if (sscanf(dimm_name, "nmem%lu", &id) == 1)
- count++;
}
- if (strcmp(argv[0], "all") == 0) {
- glob_buf.gl_pathc = ARRAY_SIZE(all);
- glob_buf.gl_pathv = all;
- } else if (!count) {
- fprintf(stderr, "Error: ' ");
- for (i = 0; i < (size_t) argc; i++)
- fprintf(stderr, "%s ", argv[i]);
- fprintf(stderr, "' does not specify any present devices\n");
- fprintf(stderr, "See 'ndctl list -D'\n");
+ if (err == argc) {
usage_with_options(u, options);
+ return -EINVAL;
}
if (param.json) {
@@ -864,23 +826,20 @@ static int dimm_action(int argc, const char **argv,
struct ndctl_ctx *ctx,
ndctl_set_log_priority(ctx, LOG_DEBUG);
rc = 0;
+ err = 0;
count = 0;
- for (i = 0; i < glob_buf.gl_pathc; i++) {
- char *dimm_name = strrchr(glob_buf.gl_pathv[i], '/');
+ for (i = 0; i < argc; i++) {
struct ndctl_dimm *dimm;
struct ndctl_bus *bus;
- unsigned long id;
- if (!dimm_name++)
- continue;
- if (sscanf(dimm_name, "nmem%lu", &id) != 1)
+ if (sscanf(argv[i], "nmem%lu", &id) != 1)
continue;
ndctl_bus_foreach(ctx, bus) {
if (!util_bus_filter(bus, param.bus))
continue;
ndctl_dimm_foreach(bus, dimm) {
- if (!util_dimm_filter(dimm, dimm_name))
+ if (!util_dimm_filter(dimm, argv[i]))
continue;
rc = action(dimm, &actx);
if (rc == 0)
@@ -902,9 +861,6 @@ static int dimm_action(int argc, const char **argv, struct
ndctl_ctx *ctx,
fclose(actx.f_out);
out:
- if (glob_cnt)
- globfree(&glob_buf);
-
/*
* count if some actions succeeded, 0 if none were attempted,
* negative error code otherwise.
