From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EE0F7EB64D9 for ; Sun, 2 Jul 2023 11:07:40 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Qv5qv20BMz3bZD for ; Sun, 2 Jul 2023 21:07:39 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linux.alibaba.com (client-ip=115.124.30.110; helo=out30-110.freemail.mail.aliyun.com; envelope-from=hsiangkao@linux.alibaba.com; receiver=lists.ozlabs.org) Received: from out30-110.freemail.mail.aliyun.com (out30-110.freemail.mail.aliyun.com [115.124.30.110]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4Qv5qp1HLJz303l for ; Sun, 2 Jul 2023 21:07:32 +1000 (AEST) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R131e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045192;MF=hsiangkao@linux.alibaba.com;NM=1;PH=DS;RN=6;SR=0;TI=SMTPD_---0VmOOYXG_1688296044; Received: from 192.168.3.2(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0VmOOYXG_1688296044) by smtp.aliyun-inc.com; Sun, 02 Jul 2023 19:07:26 +0800 Message-ID: Date: Sun, 2 Jul 2023 19:07:23 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] erofs-utils: Provide identical functionality without libuuid To: Norbert Lange , linux-erofs@lists.ozlabs.org References: <20230702101907.5081-1-nolange79@gmail.com> From: Gao Xiang In-Reply-To: <20230702101907.5081-1-nolange79@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: linux-erofs@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development of Linux EROFS file system List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Huang Jianan Errors-To: linux-erofs-bounces+linux-erofs=archiver.kernel.org@lists.ozlabs.org Sender: "Linux-erofs" 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 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 > -#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 > + */ > + > +#include > +#include > + > +#include "erofs/config.h" > +#include "erofs/defs.h" > +#include "liberofs_uuid.h" > + > +#ifdef HAVE_LIBUUID > +#include > +#else > + > +#include > +#include > + > +/* 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