* [PATCH] erofs-utils: Provide identical functionality without libuuid
@ 2023-07-02 10:19 Norbert Lange
2023-07-02 11:07 ` Gao Xiang
0 siblings, 1 reply; 4+ messages in thread
From: Norbert Lange @ 2023-07-02 10:19 UTC (permalink / raw)
To: linux-erofs; +Cc: Norbert Lange, Huang Jianan
The motivation is to have standalone (statically linked) erofs binaries
for simple initrd images, that are nevertheless able to (re)create
erofs images with a given UUID.
For this reason a few of libuuid functions have implementations added
directly in erofs-utils.
A header liberofs_uuid.h provides the new functions, which are
always available. A further sideeffect is that code can be simplified
which calls into this functionality.
The uuid_unparse function replacement is always a private
implementation and split into its own file, this further restricts
the (optional) dependency on libuuid only to the erofs-mkfs tool.
Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
dump/Makefile.am | 2 +-
dump/main.c | 8 +---
fsck/Makefile.am | 2 +-
lib/Makefile.am | 4 +-
lib/liberofs_uuid.h | 9 ++++
lib/uuid.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
lib/uuid_unparse.c | 21 +++++++++
mkfs/Makefile.am | 6 +--
mkfs/main.c | 21 ++-------
9 files changed, 149 insertions(+), 30 deletions(-)
create mode 100644 lib/liberofs_uuid.h
create mode 100644 lib/uuid.c
create mode 100644 lib/uuid_unparse.c
diff --git a/dump/Makefile.am b/dump/Makefile.am
index c2bef6d..90227a5 100644
--- a/dump/Makefile.am
+++ b/dump/Makefile.am
@@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
dump_erofs_SOURCES = main.c
dump_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
dump_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
- ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
+ ${liblz4_LIBS} ${liblzma_LIBS}
diff --git a/dump/main.c b/dump/main.c
index bc4e028..40af09f 100644
--- a/dump/main.c
+++ b/dump/main.c
@@ -17,10 +17,8 @@
#include "erofs/compress.h"
#include "erofs/fragments.h"
#include "../lib/liberofs_private.h"
+#include "../lib/liberofs_uuid.h"
-#ifdef HAVE_LIBUUID
-#include <uuid.h>
-#endif
struct erofsdump_cfg {
unsigned int totalshow;
@@ -620,9 +618,7 @@ static void erofsdump_show_superblock(void)
if (feat & feature_lists[i].flag)
fprintf(stdout, "%s ", feature_lists[i].name);
}
-#ifdef HAVE_LIBUUID
- uuid_unparse_lower(sbi.uuid, uuid_str);
-#endif
+ erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
fprintf(stdout, "\nFilesystem UUID: %s\n",
uuid_str);
}
diff --git a/fsck/Makefile.am b/fsck/Makefile.am
index e6a1fb6..4176d86 100644
--- a/fsck/Makefile.am
+++ b/fsck/Makefile.am
@@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
fsck_erofs_SOURCES = main.c
fsck_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
- ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
+ ${liblz4_LIBS} ${liblzma_LIBS}
diff --git a/lib/Makefile.am b/lib/Makefile.am
index faa7311..e243c1c 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -29,9 +29,9 @@ noinst_HEADERS += compressor.h
liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
namei.c data.c compress.c compressor.c zmap.c decompress.c \
compress_hints.c hashmap.c sha256.c blobchunk.c dir.c \
- fragments.c rb_tree.c dedupe.c
+ fragments.c rb_tree.c dedupe.c uuid_unparse.c uuid.c
-liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
+liberofs_la_CFLAGS = -Wall ${libuuid_CFLAGS} -I$(top_srcdir)/include
if ENABLE_LZ4
liberofs_la_CFLAGS += ${LZ4_CFLAGS}
liberofs_la_SOURCES += compressor_lz4.c
diff --git a/lib/liberofs_uuid.h b/lib/liberofs_uuid.h
new file mode 100644
index 0000000..d156699
--- /dev/null
+++ b/lib/liberofs_uuid.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
+#ifndef __EROFS_LIB_UUID_H
+#define __EROFS_LIB_UUID_H
+
+void erofs_uuid_generate(unsigned char *out);
+void erofs_uuid_unparse_lower(const unsigned char *buf, char *out);
+int erofs_uuid_parse(const char *in, unsigned char *uu);
+
+#endif
\ No newline at end of file
diff --git a/lib/uuid.c b/lib/uuid.c
new file mode 100644
index 0000000..acff81a
--- /dev/null
+++ b/lib/uuid.c
@@ -0,0 +1,106 @@
+// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
+/*
+ * Copyright (C) 2023 Norbert Lange <nolange79@gmail.com>
+ */
+
+#include <string.h>
+#include <errno.h>
+
+#include "erofs/config.h"
+#include "erofs/defs.h"
+#include "liberofs_uuid.h"
+
+#ifdef HAVE_LIBUUID
+#include <uuid.h>
+#else
+
+#include <stdlib.h>
+#include <sys/random.h>
+
+/* Flags to be used, will be modified if kernel does not support them */
+static unsigned erofs_grnd_flag =
+#ifdef GRND_INSECURE
+ GRND_INSECURE;
+#else
+ 0x0004;
+#endif
+
+static int s_getrandom(void *pUid, unsigned size, bool insecure)
+{
+ unsigned kflags = erofs_grnd_flag;
+ unsigned flags = insecure ? kflags : 0;
+
+ for (;;)
+ {
+ ssize_t r = getrandom(pUid, size, flags);
+ int err;
+ if (r == size)
+ break;
+ err = errno;
+ if (err != EINTR) {
+ if (err == EINVAL && kflags) {
+ // Kernel likely does not support GRND_INSECURE
+ erofs_grnd_flag = 0;
+ kflags = 0;
+ continue;
+ }
+ return -err;
+ }
+ }
+ return 0;
+}
+#endif
+
+void erofs_uuid_generate(unsigned char *pOut)
+{
+#ifdef HAVE_LIBUUID
+ uuid_t new_uuid;
+
+ do {
+ uuid_generate(new_uuid);
+ } while (uuid_is_null(new_uuid));
+#else
+ unsigned char new_uuid[16];
+ int res __maybe_unused;
+
+ res = s_getrandom(new_uuid, sizeof(new_uuid), true);
+ BUG_ON(res != 0);
+
+ // UID type + version bits
+ new_uuid[0] = (new_uuid[4 + 2] & 0x0f) | 0x40;
+ new_uuid[1] = (new_uuid[4 + 2 + 2] & 0x3f) | 0x80;
+#endif
+ memcpy(pOut, new_uuid, sizeof(new_uuid));
+}
+
+int erofs_uuid_parse(const char *in, unsigned char * uu) {
+#ifdef HAVE_LIBUUID
+ return uuid_parse((char *)in, uu);
+#else
+ unsigned char new_uuid[16];
+ unsigned hypens = ((1U << 3) | (1U << 5) | (1U << 7) | (1U << 9));
+ int i;
+
+ for (i = 0; i < sizeof(new_uuid); hypens >>= 1, i++)
+ {
+ char c[] = { in[0], in[1], '\0' };
+ char* endptr = c;
+ unsigned long val = strtoul(c, &endptr, 16);
+
+ if (endptr - c != 2)
+ return -EINVAL;
+
+ in += 2;
+
+ if ((hypens & 1U) != 0) {
+ if (*in++ != '-')
+ return -EINVAL;
+ }
+ new_uuid[i] = (unsigned char)val;
+ }
+ if (*in != '\0')
+ return -EINVAL;
+ memcpy(uu, new_uuid, sizeof(new_uuid));
+ return 0;
+#endif
+}
\ No newline at end of file
diff --git a/lib/uuid_unparse.c b/lib/uuid_unparse.c
new file mode 100644
index 0000000..762a094
--- /dev/null
+++ b/lib/uuid_unparse.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
+/*
+ * Copyright (C) 2023 Norbert Lange <nolange79@gmail.com>
+ */
+
+#include <stdio.h>
+
+#include "erofs/config.h"
+#include "liberofs_uuid.h"
+
+void erofs_uuid_unparse_lower(const unsigned char * buf, char *out) {
+ sprintf(out, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
+ (buf[0] << 8) | buf[1],
+ (buf[2] << 8) | buf[3],
+ (buf[4] << 8) | buf[5],
+ (buf[6] << 8) | buf[7],
+ (buf[8] << 8) | buf[9],
+ (buf[10] << 8) | buf[11],
+ (buf[12] << 8) | buf[13],
+ (buf[14] << 8) | buf[15]);
+}
diff --git a/mkfs/Makefile.am b/mkfs/Makefile.am
index 709d9bf..a08dc53 100644
--- a/mkfs/Makefile.am
+++ b/mkfs/Makefile.am
@@ -2,8 +2,8 @@
AUTOMAKE_OPTIONS = foreign
bin_PROGRAMS = mkfs.erofs
-AM_CPPFLAGS = ${libuuid_CFLAGS} ${libselinux_CFLAGS}
+AM_CPPFLAGS = ${libselinux_CFLAGS}
mkfs_erofs_SOURCES = main.c
mkfs_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
-mkfs_erofs_LDADD = ${libuuid_LIBS} $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
- ${liblz4_LIBS} ${liblzma_LIBS}
+mkfs_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
+ ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
diff --git a/mkfs/main.c b/mkfs/main.c
index 94f51df..840222c 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -26,10 +26,7 @@
#include "erofs/blobchunk.h"
#include "erofs/fragments.h"
#include "../lib/liberofs_private.h"
-
-#ifdef HAVE_LIBUUID
-#include <uuid.h>
-#endif
+#include "../lib/liberofs_uuid.h"
#define EROFS_SUPER_END (EROFS_SUPER_OFFSET + sizeof(struct erofs_super_block))
@@ -90,9 +87,7 @@ static void usage(void)
" -EX[,...] X=extended options\n"
" -L volume-label set the volume label (maximum 16)\n"
" -T# set a fixed UNIX timestamp # to all files\n"
-#ifdef HAVE_LIBUUID
" -UX use a given filesystem UUID\n"
-#endif
" --all-root make all files owned by root\n"
" --blobdev=X specify an extra device X to store chunked data\n"
" --chunksize=# generate chunk-based files with #-byte chunks\n"
@@ -328,14 +323,12 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
}
cfg.c_timeinherit = TIMESTAMP_FIXED;
break;
-#ifdef HAVE_LIBUUID
case 'U':
- if (uuid_parse(optarg, sbi.uuid)) {
+ if (erofs_uuid_parse(optarg, sbi.uuid)) {
erofs_err("invalid UUID %s", optarg);
return -EINVAL;
}
break;
-#endif
case 2:
opt = erofs_parse_exclude_path(optarg, false);
if (opt) {
@@ -626,11 +619,7 @@ static void erofs_mkfs_default_options(void)
EROFS_FEATURE_COMPAT_MTIME;
/* generate a default uuid first */
-#ifdef HAVE_LIBUUID
- do {
- uuid_generate(sbi.uuid);
- } while (uuid_is_null(sbi.uuid));
-#endif
+ erofs_uuid_generate(sbi.uuid);
}
/* https://reproducible-builds.org/specs/source-date-epoch/ for more details */
@@ -803,9 +792,7 @@ int main(int argc, char **argv)
erofs_strerror(err));
goto exit;
}
-#ifdef HAVE_LIBUUID
- uuid_unparse_lower(sbi.uuid, uuid_str);
-#endif
+ erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
erofs_info("filesystem UUID: %s", uuid_str);
erofs_inode_manager_init();
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] erofs-utils: Provide identical functionality without libuuid
2023-07-02 10:19 [PATCH] erofs-utils: Provide identical functionality without libuuid Norbert Lange
@ 2023-07-02 11:07 ` Gao Xiang
2023-07-02 22:16 ` Norbert Lange
0 siblings, 1 reply; 4+ messages in thread
From: Gao Xiang @ 2023-07-02 11:07 UTC (permalink / raw)
To: Norbert Lange, linux-erofs; +Cc: Huang Jianan
Hi Norbert,
On 2023/7/2 18:19, Norbert Lange wrote:
> The motivation is to have standalone (statically linked) erofs binaries
> for simple initrd images, that are nevertheless able to (re)create
> erofs images with a given UUID.
>
> For this reason a few of libuuid functions have implementations added
> directly in erofs-utils.
> A header liberofs_uuid.h provides the new functions, which are
> always available. A further sideeffect is that code can be simplified
> which calls into this functionality.
>
> The uuid_unparse function replacement is always a private
> implementation and split into its own file, this further restricts
> the (optional) dependency on libuuid only to the erofs-mkfs tool.
>
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
Yeah, overall it looks good to me, some minor nits as below:
(Also currently UUID makes the image nonreproducable, I wonder if we
could use some image hash to calculate the whole UUID instead...)
> ---
> dump/Makefile.am | 2 +-
> dump/main.c | 8 +---
> fsck/Makefile.am | 2 +-
> lib/Makefile.am | 4 +-
> lib/liberofs_uuid.h | 9 ++++
> lib/uuid.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
> lib/uuid_unparse.c | 21 +++++++++
> mkfs/Makefile.am | 6 +--
> mkfs/main.c | 21 ++-------
> 9 files changed, 149 insertions(+), 30 deletions(-)
> create mode 100644 lib/liberofs_uuid.h
> create mode 100644 lib/uuid.c
> create mode 100644 lib/uuid_unparse.c
>
> diff --git a/dump/Makefile.am b/dump/Makefile.am
> index c2bef6d..90227a5 100644
> --- a/dump/Makefile.am
> +++ b/dump/Makefile.am
> @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> dump_erofs_SOURCES = main.c
> dump_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> dump_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> + ${liblz4_LIBS} ${liblzma_LIBS}
> diff --git a/dump/main.c b/dump/main.c
> index bc4e028..40af09f 100644
> --- a/dump/main.c
> +++ b/dump/main.c
> @@ -17,10 +17,8 @@
> #include "erofs/compress.h"
> #include "erofs/fragments.h"
> #include "../lib/liberofs_private.h"
> +#include "../lib/liberofs_uuid.h"
>
> -#ifdef HAVE_LIBUUID
> -#include <uuid.h>
> -#endif
>
> struct erofsdump_cfg {
> unsigned int totalshow;
> @@ -620,9 +618,7 @@ static void erofsdump_show_superblock(void)
> if (feat & feature_lists[i].flag)
> fprintf(stdout, "%s ", feature_lists[i].name);
> }
> -#ifdef HAVE_LIBUUID
> - uuid_unparse_lower(sbi.uuid, uuid_str);
> -#endif
> + erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
> fprintf(stdout, "\nFilesystem UUID: %s\n",
> uuid_str);
> }
> diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> index e6a1fb6..4176d86 100644
> --- a/fsck/Makefile.am
> +++ b/fsck/Makefile.am
> @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> fsck_erofs_SOURCES = main.c
> fsck_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> + ${liblz4_LIBS} ${liblzma_LIBS}
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index faa7311..e243c1c 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -29,9 +29,9 @@ noinst_HEADERS += compressor.h
> liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> namei.c data.c compress.c compressor.c zmap.c decompress.c \
> compress_hints.c hashmap.c sha256.c blobchunk.c dir.c \
> - fragments.c rb_tree.c dedupe.c
> + fragments.c rb_tree.c dedupe.c uuid_unparse.c uuid.c
>
> -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> +liberofs_la_CFLAGS = -Wall ${libuuid_CFLAGS} -I$(top_srcdir)/include
> if ENABLE_LZ4
> liberofs_la_CFLAGS += ${LZ4_CFLAGS}
> liberofs_la_SOURCES += compressor_lz4.c
> diff --git a/lib/liberofs_uuid.h b/lib/liberofs_uuid.h
> new file mode 100644
> index 0000000..d156699
> --- /dev/null
> +++ b/lib/liberofs_uuid.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
> +#ifndef __EROFS_LIB_UUID_H
> +#define __EROFS_LIB_UUID_H
> +
> +void erofs_uuid_generate(unsigned char *out);
> +void erofs_uuid_unparse_lower(const unsigned char *buf, char *out);
> +int erofs_uuid_parse(const char *in, unsigned char *uu);
> +
> +#endif
> \ No newline at end of file
> diff --git a/lib/uuid.c b/lib/uuid.c
> new file mode 100644
> index 0000000..acff81a
> --- /dev/null
> +++ b/lib/uuid.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
> +/*
> + * Copyright (C) 2023 Norbert Lange <nolange79@gmail.com>
> + */
> +
> +#include <string.h>
> +#include <errno.h>
> +
> +#include "erofs/config.h"
> +#include "erofs/defs.h"
> +#include "liberofs_uuid.h"
> +
> +#ifdef HAVE_LIBUUID
> +#include <uuid.h>
> +#else
> +
> +#include <stdlib.h>
> +#include <sys/random.h>
> +
> +/* Flags to be used, will be modified if kernel does not support them */
> +static unsigned erofs_grnd_flag =
Could we switch to "unsigned int" for this?
> +#ifdef GRND_INSECURE
> + GRND_INSECURE;
> +#else
> + 0x0004;
> +#endif
> +
> +static int s_getrandom(void *pUid, unsigned size, bool insecure)
> +{
> + unsigned kflags = erofs_grnd_flag;
> + unsigned flags = insecure ? kflags : 0;
same here.
Otherwise it looks good to me.
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] erofs-utils: Provide identical functionality without libuuid
2023-07-02 11:07 ` Gao Xiang
@ 2023-07-02 22:16 ` Norbert Lange
2023-07-03 3:03 ` Gao Xiang
0 siblings, 1 reply; 4+ messages in thread
From: Norbert Lange @ 2023-07-02 22:16 UTC (permalink / raw)
To: Gao Xiang; +Cc: linux-erofs, Huang Jianan
Am So., 2. Juli 2023 um 13:07 Uhr schrieb Gao Xiang
<hsiangkao@linux.alibaba.com>:
>
> Hi Norbert,
>
> On 2023/7/2 18:19, Norbert Lange wrote:
> > The motivation is to have standalone (statically linked) erofs binaries
> > for simple initrd images, that are nevertheless able to (re)create
> > erofs images with a given UUID.
> >
> > For this reason a few of libuuid functions have implementations added
> > directly in erofs-utils.
> > A header liberofs_uuid.h provides the new functions, which are
> > always available. A further sideeffect is that code can be simplified
> > which calls into this functionality.
> >
> > The uuid_unparse function replacement is always a private
> > implementation and split into its own file, this further restricts
> > the (optional) dependency on libuuid only to the erofs-mkfs tool.
> >
> > Signed-off-by: Norbert Lange <nolange79@gmail.com>
>
> Yeah, overall it looks good to me, some minor nits as below:
>
> (Also currently UUID makes the image nonreproducable, I wonder if we
> could use some image hash to calculate the whole UUID instead...)
Not the usecase I am after, I need to use an already known UUID for
mounting, even if the content changed.
>
> > ---
> > dump/Makefile.am | 2 +-
> > dump/main.c | 8 +---
> > fsck/Makefile.am | 2 +-
> > lib/Makefile.am | 4 +-
> > lib/liberofs_uuid.h | 9 ++++
> > lib/uuid.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
> > lib/uuid_unparse.c | 21 +++++++++
> > mkfs/Makefile.am | 6 +--
> > mkfs/main.c | 21 ++-------
> > 9 files changed, 149 insertions(+), 30 deletions(-)
> > create mode 100644 lib/liberofs_uuid.h
> > create mode 100644 lib/uuid.c
> > create mode 100644 lib/uuid_unparse.c
> >
> > diff --git a/dump/Makefile.am b/dump/Makefile.am
> > index c2bef6d..90227a5 100644
> > --- a/dump/Makefile.am
> > +++ b/dump/Makefile.am
> > @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> > dump_erofs_SOURCES = main.c
> > dump_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> > dump_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> > - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> > + ${liblz4_LIBS} ${liblzma_LIBS}
> > diff --git a/dump/main.c b/dump/main.c
> > index bc4e028..40af09f 100644
> > --- a/dump/main.c
> > +++ b/dump/main.c
> > @@ -17,10 +17,8 @@
> > #include "erofs/compress.h"
> > #include "erofs/fragments.h"
> > #include "../lib/liberofs_private.h"
> > +#include "../lib/liberofs_uuid.h"
> >
> > -#ifdef HAVE_LIBUUID
> > -#include <uuid.h>
> > -#endif
> >
> > struct erofsdump_cfg {
> > unsigned int totalshow;
> > @@ -620,9 +618,7 @@ static void erofsdump_show_superblock(void)
> > if (feat & feature_lists[i].flag)
> > fprintf(stdout, "%s ", feature_lists[i].name);
> > }
> > -#ifdef HAVE_LIBUUID
> > - uuid_unparse_lower(sbi.uuid, uuid_str);
> > -#endif
> > + erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
> > fprintf(stdout, "\nFilesystem UUID: %s\n",
> > uuid_str);
> > }
> > diff --git a/fsck/Makefile.am b/fsck/Makefile.am
> > index e6a1fb6..4176d86 100644
> > --- a/fsck/Makefile.am
> > +++ b/fsck/Makefile.am
> > @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
> > fsck_erofs_SOURCES = main.c
> > fsck_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
> > fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
> > - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
> > + ${liblz4_LIBS} ${liblzma_LIBS}
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index faa7311..e243c1c 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -29,9 +29,9 @@ noinst_HEADERS += compressor.h
> > liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> > namei.c data.c compress.c compressor.c zmap.c decompress.c \
> > compress_hints.c hashmap.c sha256.c blobchunk.c dir.c \
> > - fragments.c rb_tree.c dedupe.c
> > + fragments.c rb_tree.c dedupe.c uuid_unparse.c uuid.c
> >
> > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> > +liberofs_la_CFLAGS = -Wall ${libuuid_CFLAGS} -I$(top_srcdir)/include
> > if ENABLE_LZ4
> > liberofs_la_CFLAGS += ${LZ4_CFLAGS}
> > liberofs_la_SOURCES += compressor_lz4.c
> > diff --git a/lib/liberofs_uuid.h b/lib/liberofs_uuid.h
> > new file mode 100644
> > index 0000000..d156699
> > --- /dev/null
> > +++ b/lib/liberofs_uuid.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
> > +#ifndef __EROFS_LIB_UUID_H
> > +#define __EROFS_LIB_UUID_H
> > +
> > +void erofs_uuid_generate(unsigned char *out);
> > +void erofs_uuid_unparse_lower(const unsigned char *buf, char *out);
> > +int erofs_uuid_parse(const char *in, unsigned char *uu);
> > +
> > +#endif
> > \ No newline at end of file
> > diff --git a/lib/uuid.c b/lib/uuid.c
> > new file mode 100644
> > index 0000000..acff81a
> > --- /dev/null
> > +++ b/lib/uuid.c
> > @@ -0,0 +1,106 @@
> > +// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
> > +/*
> > + * Copyright (C) 2023 Norbert Lange <nolange79@gmail.com>
> > + */
> > +
> > +#include <string.h>
> > +#include <errno.h>
> > +
> > +#include "erofs/config.h"
> > +#include "erofs/defs.h"
> > +#include "liberofs_uuid.h"
> > +
> > +#ifdef HAVE_LIBUUID
> > +#include <uuid.h>
> > +#else
> > +
> > +#include <stdlib.h>
> > +#include <sys/random.h>
> > +
> > +/* Flags to be used, will be modified if kernel does not support them */
> > +static unsigned erofs_grnd_flag =
>
> Could we switch to "unsigned int" for this?
Sure
>
> > +#ifdef GRND_INSECURE
> > + GRND_INSECURE;
> > +#else
> > + 0x0004;
> > +#endif
> > +
> > +static int s_getrandom(void *pUid, unsigned size, bool insecure)
> > +{
> > + unsigned kflags = erofs_grnd_flag;
> > + unsigned flags = insecure ? kflags : 0;
>
> same here.
>
> Otherwise it looks good to me.
Found few a formatting inconsistencies as well, already fixed this locally.
Guess I will wait a bit more for feedback before posting a v2? Not sure
whats the proper order.
>
> Thanks,
> Gao Xiang
regards, Norbert
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] erofs-utils: Provide identical functionality without libuuid
2023-07-02 22:16 ` Norbert Lange
@ 2023-07-03 3:03 ` Gao Xiang
0 siblings, 0 replies; 4+ messages in thread
From: Gao Xiang @ 2023-07-03 3:03 UTC (permalink / raw)
To: Norbert Lange; +Cc: linux-erofs, Huang Jianan
On 2023/7/3 06:16, Norbert Lange wrote:
> Am So., 2. Juli 2023 um 13:07 Uhr schrieb Gao Xiang
> <hsiangkao@linux.alibaba.com>:
>>
>> Hi Norbert,
>>
>> On 2023/7/2 18:19, Norbert Lange wrote:
>>> The motivation is to have standalone (statically linked) erofs binaries
>>> for simple initrd images, that are nevertheless able to (re)create
>>> erofs images with a given UUID.
>>>
>>> For this reason a few of libuuid functions have implementations added
>>> directly in erofs-utils.
>>> A header liberofs_uuid.h provides the new functions, which are
>>> always available. A further sideeffect is that code can be simplified
>>> which calls into this functionality.
>>>
>>> The uuid_unparse function replacement is always a private
>>> implementation and split into its own file, this further restricts
>>> the (optional) dependency on libuuid only to the erofs-mkfs tool.
>>>
>>> Signed-off-by: Norbert Lange <nolange79@gmail.com>
>>
>> Yeah, overall it looks good to me, some minor nits as below:
>>
>> (Also currently UUID makes the image nonreproducable, I wonder if we
>> could use some image hash to calculate the whole UUID instead...)
>
> Not the usecase I am after, I need to use an already known UUID for
> mounting, even if the content changed.
Okay.
>
>>
>>> ---
>>> dump/Makefile.am | 2 +-
>>> dump/main.c | 8 +---
>>> fsck/Makefile.am | 2 +-
>>> lib/Makefile.am | 4 +-
>>> lib/liberofs_uuid.h | 9 ++++
>>> lib/uuid.c | 106 ++++++++++++++++++++++++++++++++++++++++++++
>>> lib/uuid_unparse.c | 21 +++++++++
>>> mkfs/Makefile.am | 6 +--
>>> mkfs/main.c | 21 ++-------
>>> 9 files changed, 149 insertions(+), 30 deletions(-)
>>> create mode 100644 lib/liberofs_uuid.h
>>> create mode 100644 lib/uuid.c
>>> create mode 100644 lib/uuid_unparse.c
>>>
>>> diff --git a/dump/Makefile.am b/dump/Makefile.am
>>> index c2bef6d..90227a5 100644
>>> --- a/dump/Makefile.am
>>> +++ b/dump/Makefile.am
>>> @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
>>> dump_erofs_SOURCES = main.c
>>> dump_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
>>> dump_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
>>> - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
>>> + ${liblz4_LIBS} ${liblzma_LIBS}
>>> diff --git a/dump/main.c b/dump/main.c
>>> index bc4e028..40af09f 100644
>>> --- a/dump/main.c
>>> +++ b/dump/main.c
>>> @@ -17,10 +17,8 @@
>>> #include "erofs/compress.h"
>>> #include "erofs/fragments.h"
>>> #include "../lib/liberofs_private.h"
>>> +#include "../lib/liberofs_uuid.h"
>>>
>>> -#ifdef HAVE_LIBUUID
>>> -#include <uuid.h>
>>> -#endif
>>>
>>> struct erofsdump_cfg {
>>> unsigned int totalshow;
>>> @@ -620,9 +618,7 @@ static void erofsdump_show_superblock(void)
>>> if (feat & feature_lists[i].flag)
>>> fprintf(stdout, "%s ", feature_lists[i].name);
>>> }
>>> -#ifdef HAVE_LIBUUID
>>> - uuid_unparse_lower(sbi.uuid, uuid_str);
>>> -#endif
>>> + erofs_uuid_unparse_lower(sbi.uuid, uuid_str);
>>> fprintf(stdout, "\nFilesystem UUID: %s\n",
>>> uuid_str);
>>> }
>>> diff --git a/fsck/Makefile.am b/fsck/Makefile.am
>>> index e6a1fb6..4176d86 100644
>>> --- a/fsck/Makefile.am
>>> +++ b/fsck/Makefile.am
>>> @@ -7,4 +7,4 @@ AM_CPPFLAGS = ${libuuid_CFLAGS}
>>> fsck_erofs_SOURCES = main.c
>>> fsck_erofs_CFLAGS = -Wall -I$(top_srcdir)/include
>>> fsck_erofs_LDADD = $(top_builddir)/lib/liberofs.la ${libselinux_LIBS} \
>>> - ${libuuid_LIBS} ${liblz4_LIBS} ${liblzma_LIBS}
>>> + ${liblz4_LIBS} ${liblzma_LIBS}
>>> diff --git a/lib/Makefile.am b/lib/Makefile.am
>>> index faa7311..e243c1c 100644
>>> --- a/lib/Makefile.am
>>> +++ b/lib/Makefile.am
>>> @@ -29,9 +29,9 @@ noinst_HEADERS += compressor.h
>>> liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
>>> namei.c data.c compress.c compressor.c zmap.c decompress.c \
>>> compress_hints.c hashmap.c sha256.c blobchunk.c dir.c \
>>> - fragments.c rb_tree.c dedupe.c
>>> + fragments.c rb_tree.c dedupe.c uuid_unparse.c uuid.c
>>>
>>> -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
>>> +liberofs_la_CFLAGS = -Wall ${libuuid_CFLAGS} -I$(top_srcdir)/include
>>> if ENABLE_LZ4
>>> liberofs_la_CFLAGS += ${LZ4_CFLAGS}
>>> liberofs_la_SOURCES += compressor_lz4.c
>>> diff --git a/lib/liberofs_uuid.h b/lib/liberofs_uuid.h
>>> new file mode 100644
>>> index 0000000..d156699
>>> --- /dev/null
>>> +++ b/lib/liberofs_uuid.h
>>> @@ -0,0 +1,9 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0 */
>>> +#ifndef __EROFS_LIB_UUID_H
>>> +#define __EROFS_LIB_UUID_H
>>> +
>>> +void erofs_uuid_generate(unsigned char *out);
>>> +void erofs_uuid_unparse_lower(const unsigned char *buf, char *out);
>>> +int erofs_uuid_parse(const char *in, unsigned char *uu);
>>> +
>>> +#endif
>>> \ No newline at end of file
>>> diff --git a/lib/uuid.c b/lib/uuid.c
>>> new file mode 100644
>>> index 0000000..acff81a
>>> --- /dev/null
>>> +++ b/lib/uuid.c
>>> @@ -0,0 +1,106 @@
>>> +// SPDX-License-Identifier: GPL-2.0+ OR Apache-2.0
>>> +/*
>>> + * Copyright (C) 2023 Norbert Lange <nolange79@gmail.com>
>>> + */
>>> +
>>> +#include <string.h>
>>> +#include <errno.h>
>>> +
>>> +#include "erofs/config.h"
>>> +#include "erofs/defs.h"
>>> +#include "liberofs_uuid.h"
>>> +
>>> +#ifdef HAVE_LIBUUID
>>> +#include <uuid.h>
>>> +#else
>>> +
>>> +#include <stdlib.h>
>>> +#include <sys/random.h>
>>> +
>>> +/* Flags to be used, will be modified if kernel does not support them */
>>> +static unsigned erofs_grnd_flag =
>>
>> Could we switch to "unsigned int" for this?
>
> Sure
>
>>
>>> +#ifdef GRND_INSECURE
>>> + GRND_INSECURE;
>>> +#else
>>> + 0x0004;
>>> +#endif
>>> +
>>> +static int s_getrandom(void *pUid, unsigned size, bool insecure)
>>> +{
>>> + unsigned kflags = erofs_grnd_flag;
>>> + unsigned flags = insecure ? kflags : 0;
>>
>> same here.
>>
>> Otherwise it looks good to me.
>
> Found few a formatting inconsistencies as well, already fixed this locally.
> Guess I will wait a bit more for feedback before posting a v2? Not sure
> whats the proper order.
Looks good to me, yet erofs-utils has rare people to work on reviewing,
I'm fine with either way.
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-03 3:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-02 10:19 [PATCH] erofs-utils: Provide identical functionality without libuuid Norbert Lange
2023-07-02 11:07 ` Gao Xiang
2023-07-02 22:16 ` Norbert Lange
2023-07-03 3:03 ` Gao Xiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.