diff --git a/configure.in b/configure.in index 8e04f14..5ef0384 100644 --- a/configure.in +++ b/configure.in @@ -660,6 +660,10 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then fi fi AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) +if test "$with_storage_fs" = "yes"; then + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([SHOWMOUNT], ["$SHOWMOUNT"], [Location or name of the showmount program])

diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c

+static int +virStorageBackendFileSystemNetDiscoverPoolsFunc(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + char **const groups, + void *data) +{ + virNetfsDiscoverState *state = data; + virStringList *newItem; + const char *name, *path; + int len; + + path = groups[0]; + + name = rindex(path, '/');

+ if (name == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("no / in path?"));

+ return -1; + } + name += 1; + + /* Append new XML desc to list */ + + if (VIR_ALLOC(newItem) != 0) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc")); + return -1; + } + + + /* regexp pattern is too greedy, so we may have trailing spaces to trim. + * NB showmount output ambiguous for (evil) exports with names w/trailing whitespace

+ */ + len = 0; + while (name[len]) + len++;

+ while (c_isspace(name[len-1])) + len--;

+static int +virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn,

+ cleanup: + if (doc) + xmlFreeDoc(doc); + if (xpath_ctxt)

+ xmlXPathFreeContext(xpath_ctxt); + VIR_FREE(state.host); + while (state.list) { + p = state.list->next; + VIR_FREE(state.list); + state.list = p; + } + + return n_descs; +}

diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index 9a0c27f..3c16d20 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c

@@ -257,6 +258,90 @@ virStorageBackendLogicalRefreshPoolFunc(virConnectPtr conn ATTRIBUTE_UNUSED, static int +virStorageBackendLogicalDiscoverPoolsFunc(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + size_t n_tokens, + char **const tokens, + void *data) +{ + virStringList **rest = data; + virStringList *newItem; + const char *name; + int len; + + /* Append new XML desc to list */ + + if (VIR_ALLOC(newItem) != 0) { + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc")); + return -1; + } + + if (n_tokens != 1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("n_tokens should be 1")); + return -1; + } + + + name = tokens[0]; + virSkipSpaces(&name);

+ while (c_isspace(name[len-1])) + len--;

Hi David, I spotted a few things that would be good to change: David Lively <dlively(a)virtualiron.com&gt; wrote: > diff --git a/configure.in b/configure.in > index 8e04f14..5ef0384 100644 > --- a/configure.in > +++ b/configure.in > @@ -660,6 +660,10 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then > fi > fi > AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) > +if test "$with_storage_fs" = "yes"; then > + AC_PATH_PROG([SHOWMOUNT], [showmount], [], [$PATH:/sbin:/usr/sbin]) > + AC_DEFINE_UNQUOTED([SHOWMOUNT], ["$SHOWMOUNT"], [Location or name of the showmount program]) Please split long lines: AC_DEFINE_UNQUOTED([SHOWMOUNT], ["$SHOWMOUNT"], [Location or name of the showmount program]) ... > diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c ... > +static int > +virStorageBackendFileSystemNetDiscoverPoolsFunc(virConnectPtr conn ATTRIBUTE_UNUSED, > + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, > + char **const groups, > + void *data) > +{ > + virNetfsDiscoverState *state = data; > + virStringList *newItem; > + const char *name, *path; > + int len; > + > + path = groups[0]; > + > + name = rindex(path, '/'); rindex is deprecated. Using it causes portability problems. Use strrchr instead. > + if (name == NULL) { > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("no / in path?")); If you include the offending string in the diagnostic and add a word of description, it'll be more useful: virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("invalid netfs path (no slash): %s"), path); > + return -1; > + } > + name += 1; > + > + /* Append new XML desc to list */ > + > + if (VIR_ALLOC(newItem) != 0) { > + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc")); > + return -1; > + } > + > + > + /* regexp pattern is too greedy, so we may have trailing spaces to trim. > + * NB showmount output ambiguous for (evil) exports with names w/trailing whitespace Too long. > + */ > + len = 0; > + while (name[len]) > + len++; This is equivalent to the three lines above: len = strlen (name); > + while (c_isspace(name[len-1])) > + len--; It is customary to trim spaces and TABs (but less so CR, FF, NL), so c_isblank might be better than c_isspace here. ... > +static int > +virStorageBackendFileSystemNetDiscoverPools(virConnectPtr conn, ... > + cleanup: > + if (doc) > + xmlFreeDoc(doc); > + if (xpath_ctxt) You can remove this "if" test. It's unnecessary. BTW, "make syntax-check" should spot this and fail because of it. > + xmlXPathFreeContext(xpath_ctxt); > + VIR_FREE(state.host); > + while (state.list) { > + p = state.list->next; > + VIR_FREE(state.list); > + state.list = p; > + } > + > + return n_descs; > +} ... > diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c > index 9a0c27f..3c16d20 100644 > --- a/src/storage_backend_logical.c > +++ b/src/storage_backend_logical.c ... > @@ -257,6 +258,90 @@ virStorageBackendLogicalRefreshPoolFunc(virConnectPtr conn ATTRIBUTE_UNUSED, > > > static int > +virStorageBackendLogicalDiscoverPoolsFunc(virConnectPtr conn ATTRIBUTE_UNUSED, > + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, > + size_t n_tokens, > + char **const tokens, > + void *data) > +{ > + virStringList **rest = data; > + virStringList *newItem; > + const char *name; > + int len; > + > + /* Append new XML desc to list */ > + > + if (VIR_ALLOC(newItem) != 0) { > + virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("new xml desc")); > + return -1; > + } > + > + if (n_tokens != 1) { > + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("n_tokens should be 1")); > + return -1; > + } > + > + > + name = tokens[0]; > + virSkipSpaces(&name); Is is necessary to skip leading backslashes and carriage returns here? > + len = 0; > + while (name[len]) > + len++; > + while (c_isspace(name[len-1])) > + len--; A zero-length or all-"space" name would lead to referencing name[-1]. You can use this instead: while (len && c_isspace(name[len-1])) len--; The similar code above isn't vulnerable like this due to its guarantee that there is a "/".