Discussion:
[PATCH 2/6] util: Add mkdir_p implementation from testsuite
Lucas De Marchi
2013-07-15 05:12:55 UTC
Permalink
From: Lucas De Marchi <lucas.demarchi-***@public.gmane.org>

---
Makefile.am | 1 -
libkmod/libkmod-util.c | 64 ++++++++++++++++++++++++++++++++++++
libkmod/libkmod-util.h | 1 +
testsuite/init_module.c | 1 -
testsuite/mkdir.c | 86 -------------------------------------------------
testsuite/mkdir.h | 24 --------------
6 files changed, 65 insertions(+), 112 deletions(-)
delete mode 100644 testsuite/mkdir.c
delete mode 100644 testsuite/mkdir.h

diff --git a/Makefile.am b/Makefile.am
index 0ed944c..57b7372 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -154,7 +154,6 @@ testsuite_path_la_LDFLAGS = $(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_delete_module_la_LDFLAGS = $(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_init_module_la_LDFLAGS = $(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_init_module_la_SOURCES = testsuite/init_module.c \
- testsuite/mkdir.c testsuite/mkdir.h \
testsuite/stripped-module.h
testsuite_init_module_la_LIBADD = libkmod/libkmod-internal.la

diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index e636ae1..5c680e7 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -2,6 +2,8 @@
* libkmod - interface to kernel module operations
*
* Copyright (C) 2011-2013 ProFUSION embedded systems
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
+ * Copyright (C) 2012 Lucas De Marchi <lucas.de.marchi-***@public.gmane.org>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -308,6 +310,68 @@ char *path_make_absolute_cwd(const char *p)
return r;
}

+static inline int is_dir(const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st) >= 0) {
+ if (S_ISDIR(st.st_mode))
+ return 1;
+ return 0;
+ }
+
+ return -errno;
+}
+
+int mkdir_p(const char *path, mode_t mode)
+{
+ char *start = strdupa(path);
+ int len = strlen(path);
+ char *end = start + len;
+
+ /*
+ * scan backwards, replacing '/' with '\0' while the component doesn't
+ * exist
+ */
+ for (;;) {
+ int r = is_dir(start);
+ if (r > 0) {
+ end += strlen(end);
+
+ if (end == start + len)
+ return 0;
+
+ /* end != start, since it would be caught on the first
+ * iteration */
+ *end = '/';
+ break;
+ } else if (r == 0)
+ return -ENOTDIR;
+
+ if (end == start)
+ break;
+
+ *end = '\0';
+
+ /* Find the next component, backwards, discarding extra '/'*/
+ while (end > start && *end != '/')
+ end--;
+
+ while (end > start && *(end - 1) == '/')
+ end--;
+ }
+
+ for (; end < start + len;) {
+ if (mkdir(start, mode) < 0 && errno != EEXIST)
+ return -errno;
+
+ end += strlen(end);
+ *end = '/';
+ }
+
+ return 0;
+}
+
const struct kmod_ext kmod_exts[] = {
{".ko", sizeof(".ko") - 1},
#ifdef ENABLE_ZLIB
diff --git a/libkmod/libkmod-util.h b/libkmod/libkmod-util.h
index 17f8801..83c975c 100644
--- a/libkmod/libkmod-util.h
+++ b/libkmod/libkmod-util.h
@@ -20,6 +20,7 @@ int read_str_ulong(int fd, unsigned long *value, int base) _must_check_ __attrib
char *strchr_replace(char *s, int c, char r);
bool path_is_absolute(const char *p) _must_check_ __attribute__((nonnull(1)));
char *path_make_absolute_cwd(const char *p) _must_check_ __attribute__((nonnull(1)));
+int mkdir_p(const char *path, mode_t mode);
int alias_normalize(const char *alias, char buf[PATH_MAX], size_t *len) _must_check_ __attribute__((nonnull(1,2)));
char *modname_normalize(const char *modname, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(1, 2)));
char *path_to_modname(const char *path, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(2)));
diff --git a/testsuite/init_module.c b/testsuite/init_module.c
index 686f671..ebf1b94 100644
--- a/testsuite/init_module.c
+++ b/testsuite/init_module.c
@@ -44,7 +44,6 @@

/* FIXME: hack, change name so we don't clash */
#undef ERR
-#include "mkdir.h"
#include "testsuite.h"
#include "stripped-module.h"

diff --git a/testsuite/mkdir.c b/testsuite/mkdir.c
deleted file mode 100644
index be2e37b..0000000
--- a/testsuite/mkdir.c
+++ /dev/null
@@ -1,86 +0,0 @@
-/*
- * Copyright (C) 2012 Lucas De Marchi <lucas.de.marchi-***@public.gmane.org
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include <errno.h>
-#include <string.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-
-#include "mkdir.h"
-#include "testsuite.h"
-
-static inline int is_dir(const char *path)
-{
- struct stat st;
-
- if (stat(path, &st) >= 0) {
- if (S_ISDIR(st.st_mode))
- return 1;
- return 0;
- }
-
- return -errno;
-}
-
-TS_EXPORT int mkdir_p(const char *path, mode_t mode)
-{
- char *start = strdupa(path);
- int len = strlen(path);
- char *end = start + len;
-
- /*
- * scan backwards, replacing '/' with '\0' while the component doesn't
- * exist
- */
- for (;;) {
- int r = is_dir(start);
- if (r > 0) {
- end += strlen(end);
-
- if (end == start + len)
- return 0;
-
- /* end != start, since it would be caught on the first
- * iteration */
- *end = '/';
- break;
- } else if (r == 0)
- return -ENOTDIR;
-
- if (end == start)
- break;
-
- *end = '\0';
-
- /* Find the next component, backwards, discarding extra '/'*/
- while (end > start && *end != '/')
- end--;
-
- while (end > start && *(end - 1) == '/')
- end--;
- }
-
- for (; end < start + len;) {
- if (mkdir(start, mode) < 0 && errno != EEXIST)
- return -errno;
-
- end += strlen(end);
- *end = '/';
- }
-
- return 0;
-}
diff --git a/testsuite/mkdir.h b/testsuite/mkdir.h
deleted file mode 100644
index 35f6a1d..0000000
--- a/testsuite/mkdir.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * Copyright (C) 2012 Lucas De Marchi <lucas.de.marchi-***@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#pragma once
-
-#include <sys/stat.h>
-#include <sys/types.h>
-
-int mkdir_p(const char *path, mode_t mode);
--
1.8.3.2
Lucas De Marchi
2013-07-15 05:12:56 UTC
Permalink
From: Tom Gundersen <teg-***@public.gmane.org>

In containers/VM's/initrd one might not have installed any modules and
accompanying modules.devname Don't fail if this is the case, just warn.

When used in systemd this means we don't get a failing unit on booting
containers.
---
tools/static-nodes.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/static-nodes.c b/tools/static-nodes.c
index 96bb71e..351c02b 100644
--- a/tools/static-nodes.c
+++ b/tools/static-nodes.c
@@ -155,7 +155,8 @@ static int do_static_nodes(int argc, char *argv[])
{
struct utsname kernel;
char modules[PATH_MAX];
- FILE *in = NULL, *out = stdout;
+ const char *output = "/dev/stdout";
+ FILE *in = NULL, *out = NULL;
const struct static_nodes_format *format = &static_nodes_format_human;
char buf[4096];
int ret = EXIT_SUCCESS;
@@ -170,13 +171,7 @@ static int do_static_nodes(int argc, char *argv[])
}
switch (c) {
case 'o':
- out = fopen(optarg, "we");
- if (out == NULL) {
- fprintf(stderr, "Error: could not create %s!\n",
- optarg);
- ret = EXIT_FAILURE;
- goto finish;
- }
+ output = optarg;
break;
case 'f':
valid = 0;
@@ -217,12 +212,24 @@ static int do_static_nodes(int argc, char *argv[])
goto finish;
}

- snprintf(modules, sizeof(modules), "/lib/modules/%s/modules.devname",
- kernel.release);
+ snprintf(modules, sizeof(modules), "/lib/modules/%s/modules.devname", kernel.release);
in = fopen(modules, "re");
if (in == NULL) {
- fprintf(stderr, "Error: could not open /lib/modules/%s/modules.devname - %m\n",
- kernel.release);
+ if (errno == ENOENT) {
+ fprintf(stderr, "Warning: /lib/modules/%s/modules.devname not found - ignoring\n",
+ kernel.release);
+ ret = EXIT_SUCCESS;
+ } else {
+ fprintf(stderr, "Error: could not open /lib/modules/%s/modules.devname - %m\n",
+ kernel.release);
+ ret = EXIT_FAILURE;
+ }
+ goto finish;
+ }
+
+ out = fopen(output, "we");
+ if (out == NULL) {
+ fprintf(stderr, "Error: could not create %s - %m\n", output);
ret = EXIT_FAILURE;
goto finish;
}
--
1.8.3.2
Lucas De Marchi
2013-07-15 05:12:59 UTC
Permalink
From: Tom Gundersen <teg-***@public.gmane.org>

Allows us to drop call to "mkdir -p" from the systemd service file.
---
tools/static-nodes.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/static-nodes.c b/tools/static-nodes.c
index 351c02b..0195390 100644
--- a/tools/static-nodes.c
+++ b/tools/static-nodes.c
@@ -154,12 +154,11 @@ static void help(void)
static int do_static_nodes(int argc, char *argv[])
{
struct utsname kernel;
- char modules[PATH_MAX];
+ char modules[PATH_MAX], buf[4096];
const char *output = "/dev/stdout";
FILE *in = NULL, *out = NULL;
const struct static_nodes_format *format = &static_nodes_format_human;
- char buf[4096];
- int ret = EXIT_SUCCESS;
+ int r, ret = EXIT_SUCCESS;

for (;;) {
int c, idx = 0, valid;
@@ -227,6 +226,13 @@ static int do_static_nodes(int argc, char *argv[])
goto finish;
}

+ r = mkdir_parents(output, 0755);
+ if (r < 0) {
+ fprintf(stderr, "Error: could not create parent directory for %s - %m.\n", output);
+ ret = EXIT_FAILURE;
+ goto finish;
+ }
+
out = fopen(output, "we");
if (out == NULL) {
fprintf(stderr, "Error: could not create %s - %m\n", output);
--
1.8.3.2
Lucas De Marchi
2013-07-15 05:12:57 UTC
Permalink
From: Lucas De Marchi <lucas.demarchi-***@public.gmane.org>

---
libkmod/libkmod-util.c | 9 +++++----
libkmod/libkmod-util.h | 2 +-
testsuite/init_module.c | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index 5c680e7..45ca11a 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -323,11 +323,12 @@ static inline int is_dir(const char *path)
return -errno;
}

-int mkdir_p(const char *path, mode_t mode)
+int mkdir_p(const char *path, int len, mode_t mode)
{
- char *start = strdupa(path);
- int len = strlen(path);
- char *end = start + len;
+ char *start, *end;
+
+ start = strndupa(path, len);
+ end = start + len;

/*
* scan backwards, replacing '/' with '\0' while the component doesn't
diff --git a/libkmod/libkmod-util.h b/libkmod/libkmod-util.h
index 83c975c..4c45416 100644
--- a/libkmod/libkmod-util.h
+++ b/libkmod/libkmod-util.h
@@ -20,7 +20,7 @@ int read_str_ulong(int fd, unsigned long *value, int base) _must_check_ __attrib
char *strchr_replace(char *s, int c, char r);
bool path_is_absolute(const char *p) _must_check_ __attribute__((nonnull(1)));
char *path_make_absolute_cwd(const char *p) _must_check_ __attribute__((nonnull(1)));
-int mkdir_p(const char *path, mode_t mode);
+int mkdir_p(const char *path, int len, mode_t mode);
int alias_normalize(const char *alias, char buf[PATH_MAX], size_t *len) _must_check_ __attribute__((nonnull(1,2)));
char *modname_normalize(const char *modname, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(1, 2)));
char *path_to_modname(const char *path, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(2)));
diff --git a/testsuite/init_module.c b/testsuite/init_module.c
index ebf1b94..42177e7 100644
--- a/testsuite/init_module.c
+++ b/testsuite/init_module.c
@@ -155,7 +155,7 @@ static int create_sysfs_files(const char *modname)
strcpy(buf + len, modname);
len += strlen(modname);

- assert(mkdir_p(buf, 0755) >= 0);
+ assert(mkdir_p(buf, len, 0755) >= 0);

strcpy(buf + len, "/initstate");
return write_one_line_file(buf, "live\n", strlen("live\n"));
--
1.8.3.2
Lucas De Marchi
2013-07-15 05:12:58 UTC
Permalink
From: Lucas De Marchi <lucas.demarchi-***@public.gmane.org>

Like mkdir_p, but discard the leaf and create all parent directories.
---
libkmod/libkmod-util.c | 11 +++++++++++
libkmod/libkmod-util.h | 1 +
2 files changed, 12 insertions(+)

diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index 45ca11a..764c551 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -373,6 +373,17 @@ int mkdir_p(const char *path, int len, mode_t mode)
return 0;
}

+int mkdir_parents(const char *path, mode_t mode)
+{
+ char *end = strrchr(path, '/');
+
+ /* no parent directories */
+ if (end == NULL)
+ return 0;
+
+ return mkdir_p(path, end - path, mode);
+}
+
const struct kmod_ext kmod_exts[] = {
{".ko", sizeof(".ko") - 1},
#ifdef ENABLE_ZLIB
diff --git a/libkmod/libkmod-util.h b/libkmod/libkmod-util.h
index 4c45416..f7f3e90 100644
--- a/libkmod/libkmod-util.h
+++ b/libkmod/libkmod-util.h
@@ -21,6 +21,7 @@ char *strchr_replace(char *s, int c, char r);
bool path_is_absolute(const char *p) _must_check_ __attribute__((nonnull(1)));
char *path_make_absolute_cwd(const char *p) _must_check_ __attribute__((nonnull(1)));
int mkdir_p(const char *path, int len, mode_t mode);
+int mkdir_parents(const char *path, mode_t mode);
int alias_normalize(const char *alias, char buf[PATH_MAX], size_t *len) _must_check_ __attribute__((nonnull(1,2)));
char *modname_normalize(const char *modname, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(1, 2)));
char *path_to_modname(const char *path, char buf[PATH_MAX], size_t *len) __attribute__((nonnull(2)));
--
1.8.3.2
Tom Gundersen
2013-07-15 15:19:29 UTC
Permalink
Hi Lucas,

Thanks for picking this up.

All the patches look good to me, and "kmod static-nodes" now works as expected.

Cheers,

Tom

On Mon, Jul 15, 2013 at 7:12 AM, Lucas De Marchi
- Fix infinite loop when path is relative
- Fix not considering EEXIST as a success
- General refactor to mkdir_p so it never calls mkdir for an existing
dir (given no creates it from outside)
---
testsuite/mkdir.c | 54 +++++++++++++++++++++++++++++++++++-------------------
1 file changed, 35 insertions(+), 19 deletions(-)
diff --git a/testsuite/mkdir.c b/testsuite/mkdir.c
index 0a7de69..be2e37b 100644
--- a/testsuite/mkdir.c
+++ b/testsuite/mkdir.c
@@ -23,47 +23,63 @@
#include "mkdir.h"
#include "testsuite.h"
+static inline int is_dir(const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st) >= 0) {
+ if (S_ISDIR(st.st_mode))
+ return 1;
+ return 0;
+ }
+
+ return -errno;
+}
+
TS_EXPORT int mkdir_p(const char *path, mode_t mode)
{
char *start = strdupa(path);
int len = strlen(path);
char *end = start + len;
- struct stat st;
/*
* scan backwards, replacing '/' with '\0' while the component doesn't
* exist
*/
for (;;) {
- if (stat(start, &st) >= 0) {
- if (S_ISDIR(st.st_mode))
- break;
- return -ENOTDIR;
- }
+ int r = is_dir(start);
+ if (r > 0) {
+ end += strlen(end);
- /* Find the next component, backwards, discarding extra '/'*/
- for (; end != start && *end != '/'; end--)
- ;
+ if (end == start + len)
+ return 0;
- for (; end != start - 1 && *end == '/'; end--)
- ;
+ /* end != start, since it would be caught on the first
+ * iteration */
+ *end = '/';
+ break;
+ } else if (r == 0)
+ return -ENOTDIR;
- end++;
if (end == start)
break;
*end = '\0';
- }
- if (end == start + len)
- return 0;
+ /* Find the next component, backwards, discarding extra '/'*/
+ while (end > start && *end != '/')
+ end--;
- for (; end < start + len;) {
- *end = '/';
- end += strlen(end);
+ while (end > start && *(end - 1) == '/')
+ end--;
+ }
- if (mkdir(start, mode) < 0)
+ for (; end < start + len;) {
+ if (mkdir(start, mode) < 0 && errno != EEXIST)
return -errno;
+
+ end += strlen(end);
+ *end = '/';
}
return 0;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
Lucas De Marchi
2013-07-15 15:39:27 UTC
Permalink
Post by Lucas De Marchi
---
Makefile.am | 1 -
libkmod/libkmod-util.c | 64 ++++++++++++++++++++++++++++++++++++
libkmod/libkmod-util.h | 1 +
testsuite/init_module.c | 1 -
testsuite/mkdir.c | 86
-------------------------------------------------
testsuite/mkdir.h | 24 --------------
6 files changed, 65 insertions(+), 112 deletions(-)
delete mode 100644 testsuite/mkdir.c
delete mode 100644 testsuite/mkdir.h
diff --git a/Makefile.am b/Makefile.am
index 0ed944c..57b7372 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -154,7 +154,6 @@ testsuite_path_la_LDFLAGS =
$(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_delete_module_la_LDFLAGS = $(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_init_module_la_LDFLAGS = $(TESTSUITE_OVERRIDE_LIBS_LDFLAGS)
testsuite_init_module_la_SOURCES = testsuite/init_module.c \
- testsuite/mkdir.c testsuite/mkdir.h \
testsuite/stripped-module.h
testsuite_init_module_la_LIBADD = libkmod/libkmod-internal.la
diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index e636ae1..5c680e7 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -2,6 +2,8 @@
* libkmod - interface to kernel module operations
*
* Copyright (C) 2011-2013 ProFUSION embedded systems
+ * Copyright (C) 2013 Intel Corporation. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -308,6 +310,68 @@ char *path_make_absolute_cwd(const char *p)
return r;
}
+static inline int is_dir(const char *path)
+{
+ struct stat st;
+
+ if (stat(path, &st) >= 0) {
+ if (S_ISDIR(st.st_mode))
+ return 1;
+ return 0;
Why not just return S_ISDIR(st.st_mode);
I can just blame lack of caffeine ;-). I will change it before pushing.

Thanks
Lucas De Marchi

Loading...