git: f0ac5e919f3f - main - fts: Add blocks support.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 22 Apr 2025 17:26:00 UTC
The branch main has been updated by des: URL: https://cgit.FreeBSD.org/src/commit/?id=f0ac5e919f3f9918075d1a6da26d12e675e45b11 commit f0ac5e919f3f9918075d1a6da26d12e675e45b11 Author: Dag-Erling Smørgrav <des@FreeBSD.org> AuthorDate: 2025-04-22 17:16:59 +0000 Commit: Dag-Erling Smørgrav <des@FreeBSD.org> CommitDate: 2025-04-22 17:25:56 +0000 fts: Add blocks support. This adds an `fts_open_b()` variant of `fts_open()` which takes a block instead of a function pointer. This was inspired by, and is intended to be compatible with, Apple's implementation; however, although our FTS and theirs share a common ancestor, they have diverged significantly. That and the fact that we still target compilers which don't support blocks means Apple's implementation was not directly reusable. This is the second use case for blocks in FreeBSD (the first being `qsort_b()`, which we use here). This suggest we might want to add a `COMPILER_FEATURE` for blocks to avoid hardcoding any further `COMPILER_TYPE` checks. MFC after: never Relnotes: yes Sponsored by: Klara, Inc. Reviewed by: kevans, theraven, imp Differential Revision: https://reviews.freebsd.org/D49877 --- include/fts.h | 29 ++++-- lib/libc/gen/Makefile.inc | 4 + lib/libc/gen/Symbol.map | 1 + lib/libc/gen/fts.3 | 58 +++++++++--- lib/libc/gen/fts.c | 172 ++++++++++++++++++++++++++--------- lib/libc/tests/gen/Makefile | 9 ++ lib/libc/tests/gen/fts_blocks_test.c | 63 +++++++++++++ lib/libc/tests/stdlib/Makefile | 2 +- 8 files changed, 275 insertions(+), 63 deletions(-) diff --git a/include/fts.h b/include/fts.h index 1e35727ad3e1..f2c40b854ffb 100644 --- a/include/fts.h +++ b/include/fts.h @@ -34,17 +34,27 @@ #include <sys/_types.h> +typedef struct _ftsent FTSENT; + typedef struct { - struct _ftsent *fts_cur; /* current node */ - struct _ftsent *fts_child; /* linked list of children */ - struct _ftsent **fts_array; /* sort array */ + FTSENT *fts_cur; /* current node */ + FTSENT *fts_child; /* linked list of children */ + FTSENT **fts_array; /* sort array */ __dev_t fts_dev; /* starting device # */ char *fts_path; /* path for this descent */ int fts_rfd; /* fd for root */ __size_t fts_pathlen; /* sizeof(path) */ __size_t fts_nitems; /* elements in the sort array */ - int (*fts_compar) /* compare function */ - (const struct _ftsent * const *, const struct _ftsent * const *); + union { + int (*fts_compar) /* compare function */ + (const FTSENT * const *, const FTSENT * const *); +#ifdef __BLOCKS__ + int (^fts_compar_b) + (const FTSENT * const *, const FTSENT * const *); +#else + void *fts_compar_b; +#endif /* __BLOCKS__ */ + }; /* valid for fts_open() */ #define FTS_COMFOLLOW 0x000001 /* follow command line symlinks */ @@ -62,11 +72,12 @@ typedef struct { /* internal use only */ #define FTS_STOP 0x010000 /* unrecoverable error */ +#define FTS_COMPAR_B 0x020000 /* compare function is a block */ int fts_options; /* fts_open options, global flags */ void *fts_clientptr; /* thunk for sort function */ } FTS; -typedef struct _ftsent { +struct _ftsent { struct _ftsent *fts_cycle; /* cycle node */ struct _ftsent *fts_parent; /* parent directory */ struct _ftsent *fts_link; /* next file in directory */ @@ -118,7 +129,7 @@ typedef struct _ftsent { struct stat *fts_statp; /* stat(2) information */ char *fts_name; /* file name */ FTS *fts_fts; /* back pointer to main FTS */ -} FTSENT; +}; #include <sys/cdefs.h> @@ -131,6 +142,10 @@ FTS *fts_get_stream(FTSENT *); #define fts_get_stream(ftsent) ((ftsent)->fts_fts) FTS *fts_open(char * const *, int, int (*)(const FTSENT * const *, const FTSENT * const *)); +#ifdef __BLOCKS__ +FTS *fts_open_b(char * const *, int, + int (^)(const FTSENT * const *, const FTSENT * const *)); +#endif /* __BLOCKS__ */ FTSENT *fts_read(FTS *); int fts_set(FTS *, FTSENT *, int); void fts_set_clientptr(FTS *, void *); diff --git a/lib/libc/gen/Makefile.inc b/lib/libc/gen/Makefile.inc index a8308a057b05..fb2f0afaa2c7 100644 --- a/lib/libc/gen/Makefile.inc +++ b/lib/libc/gen/Makefile.inc @@ -168,6 +168,10 @@ SRCS+= \ valloc.c \ wordexp.c +.if ${COMPILER_TYPE} == "clang" +CFLAGS.fts.c= -fblocks +.endif + CFLAGS.arc4random.c= -I${SRCTOP}/sys -I${SRCTOP}/sys/crypto/chacha20 CFLAGS.sysconf.c= -I${SRCTOP}/contrib/tzcode diff --git a/lib/libc/gen/Symbol.map b/lib/libc/gen/Symbol.map index 21b66acba213..1683b3e42425 100644 --- a/lib/libc/gen/Symbol.map +++ b/lib/libc/gen/Symbol.map @@ -458,6 +458,7 @@ FBSD_1.8 { aio_read2; aio_write2; execvpe; + fts_open_b; psiginfo; rtld_get_var; rtld_set_var; diff --git a/lib/libc/gen/fts.3 b/lib/libc/gen/fts.3 index 468b14115ec6..3007b773ec55 100644 --- a/lib/libc/gen/fts.3 +++ b/lib/libc/gen/fts.3 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd January 12, 2014 +.Dd April 17, 2025 .Dt FTS 3 .Os .Sh NAME @@ -37,6 +37,8 @@ .In fts.h .Ft FTS * .Fn fts_open "char * const *path_argv" "int options" "int (*compar)(const FTSENT * const *, const FTSENT * const *)" +.Ft FTS * +.Fn fts_open_b "char * const *path_argv" "int options" "int (^compar)(const FTSENT * const *, const FTSENT * const *)" .Ft FTSENT * .Fn fts_read "FTS *ftsp" .Ft FTSENT * @@ -59,7 +61,9 @@ functions are provided for traversing file hierarchies. A simple overview is that the .Fn fts_open -function returns a +and +.Fn fts_open_b +functions return a .Dq handle on a file hierarchy, which is then supplied to the other @@ -186,6 +190,8 @@ or .Ql ..\& which was not specified as a file name to .Fn fts_open +or +.Fn fts_open_b (see .Dv FTS_SEEDOT ) . .It Dv FTS_DP @@ -234,6 +240,8 @@ A path for accessing the file from the current directory. The path for the file relative to the root of the traversal. This path contains the path specified to .Fn fts_open +or +.Fn fts_open_b as a prefix. .It Fa fts_pathlen The length of the string referenced by @@ -518,6 +526,15 @@ the directory traversal order is in the order listed in .Fa path_argv for the root paths, and in the order listed in the directory for everything else. +.Sh FTS_OPEN_B +The +.Fn fts_open_b +function is identical to +.Fn fts_open +except that it takes a block pointer instead of a function pointer. +The block is copied before +.Fn fts_open_b +returns, so the original can safely go out of scope or be released. .Sh FTS_READ The .Fn fts_read @@ -593,9 +610,13 @@ As a special case, if has not yet been called for a hierarchy, .Fn fts_children will return a pointer to the files in the logical directory specified to -.Fn fts_open , +.Fn fts_open +or +.Fn fts_open_b , i.e., the arguments specified to -.Fn fts_open . +.Fn fts_open +or +.Fn fts_open_b . Otherwise, if the .Vt FTSENT structure most recently returned by @@ -716,6 +737,8 @@ function closes a file hierarchy stream .Fa ftsp and restores the current directory to the directory from which .Fn fts_open +or +.Fn fts_open_b was called to open .Fa ftsp . The @@ -723,29 +746,38 @@ The function returns 0 on success, and \-1 if an error occurs. .Sh ERRORS -The function +The .Fn fts_open -may fail and set +and +.Fn fts_open_b +functions may fail and set .Va errno for any of the errors specified for the library functions .Xr open 2 and .Xr malloc 3 . +The +.Fn fts_open_b +function may also fail and set +.Va errno +to +.Dv ENOSYS +if the blocks runtime is missing. .Pp -The function +The .Fn fts_close -may fail and set +function may fail and set .Va errno for any of the errors specified for the library functions .Xr chdir 2 and .Xr close 2 . .Pp -The functions +The .Fn fts_read and .Fn fts_children -may fail and set +functions may fail and set .Va errno for any of the errors specified for the library functions .Xr chdir 2 , @@ -755,12 +787,12 @@ for any of the errors specified for the library functions and .Xr stat 2 . .Pp -In addition, +In addition, the .Fn fts_children , -.Fn fts_open +.Fn fts_open , and .Fn fts_set -may fail and set +functions may fail and set .Va errno as follows: .Bl -tag -width Er diff --git a/lib/libc/gen/fts.c b/lib/libc/gen/fts.c index 0c26c91feb84..1b5c38ed58bb 100644 --- a/lib/libc/gen/fts.c +++ b/lib/libc/gen/fts.c @@ -48,6 +48,19 @@ #include "gen-private.h" +#ifdef __BLOCKS__ +#include <Block.h> +#else +#include "block_abi.h" +typedef DECLARE_BLOCK(int, fts_block, + const FTSENT * const *, const FTSENT * const *); +void qsort_b(void *, size_t, size_t, fts_block); +#endif /* __BLOCKS__ */ +/* only present if linked with blocks runtime */ +void *_Block_copy(const void *) __weak_symbol; +void _Block_release(const void *) __weak_symbol; +extern void *_NSConcreteGlobalBlock[] __weak_symbol; + static FTSENT *fts_alloc(FTS *, char *, size_t); static FTSENT *fts_build(FTS *, int); static void fts_lfree(FTSENT *); @@ -102,35 +115,13 @@ static const char *ufslike_filesystems[] = { 0 }; -FTS * -fts_open(char * const *argv, int options, - int (*compar)(const FTSENT * const *, const FTSENT * const *)) +static FTS * +__fts_open(FTS *sp, char * const *argv) { - struct _fts_private *priv; - FTS *sp; FTSENT *p, *root; FTSENT *parent, *tmp; size_t len, nitems; - /* Options check. */ - if (options & ~FTS_OPTIONMASK) { - errno = EINVAL; - return (NULL); - } - - /* fts_open() requires at least one path */ - if (*argv == NULL) { - errno = EINVAL; - return (NULL); - } - - /* Allocate/initialize the stream. */ - if ((priv = calloc(1, sizeof(*priv))) == NULL) - return (NULL); - sp = &priv->ftsp_fts; - sp->fts_compar = compar; - sp->fts_options = options; - /* Logical walks turn on NOCHDIR; symbolic links are too hard. */ if (ISSET(FTS_LOGICAL)) SET(FTS_NOCHDIR); @@ -168,7 +159,7 @@ fts_open(char * const *argv, int options, * If comparison routine supplied, traverse in sorted * order; otherwise traverse in the order specified. */ - if (compar) { + if (sp->fts_compar) { p->fts_link = root; root = p; } else { @@ -181,7 +172,7 @@ fts_open(char * const *argv, int options, } } } - if (compar && nitems > 1) + if (sp->fts_compar && nitems > 1) root = fts_sort(sp, root, nitems); /* @@ -214,6 +205,97 @@ mem1: free(sp); return (NULL); } +FTS * +fts_open(char * const *argv, int options, + int (*compar)(const FTSENT * const *, const FTSENT * const *)) +{ + struct _fts_private *priv; + FTS *sp; + + /* Options check. */ + if (options & ~FTS_OPTIONMASK) { + errno = EINVAL; + return (NULL); + } + + /* fts_open() requires at least one path */ + if (*argv == NULL) { + errno = EINVAL; + return (NULL); + } + + /* Allocate/initialize the stream. */ + if ((priv = calloc(1, sizeof(*priv))) == NULL) + return (NULL); + sp = &priv->ftsp_fts; + sp->fts_compar = compar; + sp->fts_options = options; + + return (__fts_open(sp, argv)); +} + +#ifdef __BLOCKS__ +FTS * +fts_open_b(char * const *argv, int options, + int (^compar)(const FTSENT * const *, const FTSENT * const *)) +#else +FTS * +fts_open_b(char * const *argv, int options, fts_block compar) +#endif /* __BLOCKS__ */ +{ + struct _fts_private *priv; + FTS *sp; + + /* No blocks, no problems. */ + if (compar == NULL) + return (fts_open(argv, options, NULL)); + + /* Avoid segfault if blocks runtime is missing. */ + if (_Block_copy == NULL) { + errno = ENOSYS; + return (NULL); + } + + /* Options check. */ + if (options & ~FTS_OPTIONMASK) { + errno = EINVAL; + return (NULL); + } + + /* fts_open() requires at least one path */ + if (*argv == NULL) { + errno = EINVAL; + return (NULL); + } + + /* Allocate/initialize the stream. */ + if ((priv = calloc(1, sizeof(*priv))) == NULL) + return (NULL); + sp = &priv->ftsp_fts; +#ifdef __BLOCKS__ + compar = Block_copy(compar); +#else + if (compar->isa != &_NSConcreteGlobalBlock) + compar = _Block_copy(compar); +#endif /* __BLOCKS__ */ + if (compar == NULL) { + free(priv); + return (NULL); + } + sp->fts_compar_b = compar; + sp->fts_options = options | FTS_COMPAR_B; + + if ((sp = __fts_open(sp, argv)) == NULL) { +#ifdef __BLOCKS__ + Block_release(compar); +#else + if (compar->isa != &_NSConcreteGlobalBlock) + _Block_release(compar); +#endif /* __BLOCKS__ */ + } + return (sp); +} + static void fts_load(FTS *sp, FTSENT *p) { @@ -265,6 +347,16 @@ fts_close(FTS *sp) free(sp->fts_array); free(sp->fts_path); + /* Free up any block pointer. */ + if (ISSET(FTS_COMPAR_B) && sp->fts_compar_b != NULL) { +#ifdef __BLOCKS__ + Block_release(sp->fts_compar_b); +#else + if (sp->fts_compar_b->isa != &_NSConcreteGlobalBlock) + _Block_release(sp->fts_compar_b); +#endif /* __BLOCKS__ */ + } + /* Return to original directory, save errno if necessary. */ if (!ISSET(FTS_NOCHDIR)) { saved_errno = fchdir(sp->fts_rfd) ? errno : 0; @@ -979,21 +1071,6 @@ err: memset(sbp, 0, sizeof(struct stat)); return (FTS_DEFAULT); } -/* - * The comparison function takes pointers to pointers to FTSENT structures. - * Qsort wants a comparison function that takes pointers to void. - * (Both with appropriate levels of const-poisoning, of course!) - * Use a trampoline function to deal with the difference. - */ -static int -fts_compar(const void *a, const void *b) -{ - FTS *parent; - - parent = (*(const FTSENT * const *)a)->fts_fts; - return (*parent->fts_compar)(a, b); -} - static FTSENT * fts_sort(FTS *sp, FTSENT *head, size_t nitems) { @@ -1016,7 +1093,18 @@ fts_sort(FTS *sp, FTSENT *head, size_t nitems) } for (ap = sp->fts_array, p = head; p; p = p->fts_link) *ap++ = p; - qsort(sp->fts_array, nitems, sizeof(FTSENT *), fts_compar); + if (ISSET(FTS_COMPAR_B)) { +#ifdef __BLOCKS__ + qsort_b(sp->fts_array, nitems, sizeof(FTSENT *), + (int (^)(const void *, const void *))sp->fts_compar_b); +#else + qsort_b(sp->fts_array, nitems, sizeof(FTSENT *), + sp->fts_compar_b); +#endif /* __BLOCKS__ */ + } else { + qsort(sp->fts_array, nitems, sizeof(FTSENT *), + (int (*)(const void *, const void *))sp->fts_compar); + } for (head = *(ap = sp->fts_array); --nitems; ++ap) ap[0]->fts_link = ap[1]; ap[0]->fts_link = NULL; diff --git a/lib/libc/tests/gen/Makefile b/lib/libc/tests/gen/Makefile index 87f2b8320d25..5c0febce81c1 100644 --- a/lib/libc/tests/gen/Makefile +++ b/lib/libc/tests/gen/Makefile @@ -7,6 +7,9 @@ ATF_TESTS_C+= fmtcheck2_test ATF_TESTS_C+= fmtmsg_test ATF_TESTS_C+= fnmatch2_test ATF_TESTS_C+= fpclassify2_test +.if ${COMPILER_TYPE} == "clang" +ATF_TESTS_C+= fts_blocks_test +.endif ATF_TESTS_C+= ftw_test ATF_TESTS_C+= getentropy_test ATF_TESTS_C+= getmntinfo_test @@ -92,6 +95,12 @@ TEST_METADATA.setdomainname_test+= is_exclusive=true TESTS_SUBDIRS= execve TESTS_SUBDIRS+= posix_spawn +# Tests that require blocks support +.for t in fts_blocks_test +CFLAGS.${t}.c+= -fblocks +LIBADD.${t}+= BlocksRuntime +.endfor + # The old testcase name TEST_FNMATCH= test-fnmatch CLEANFILES+= ${GEN_SH_CASE_TESTCASES} diff --git a/lib/libc/tests/gen/fts_blocks_test.c b/lib/libc/tests/gen/fts_blocks_test.c new file mode 100644 index 000000000000..1aec55b581d5 --- /dev/null +++ b/lib/libc/tests/gen/fts_blocks_test.c @@ -0,0 +1,63 @@ +/*- + * Copyright (c) 2025 Klara, Inc. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include <sys/stat.h> + +#include <fcntl.h> +#include <fts.h> + +#include <atf-c.h> + +/* + * Create two directories with three files each in lexicographical order, + * then call FTS with a sort block that sorts in reverse lexicographical + * order. This has the least chance of getting a false positive due to + * differing file system semantics. UFS will return the files in the + * order they were created while ZFS will sort them lexicographically; in + * both cases, the order we expect is the reverse. + */ +ATF_TC_WITHOUT_HEAD(fts_blocks_test); +ATF_TC_BODY(fts_blocks_test, tc) +{ + char *args[] = { + "bar", "foo", NULL + }; + char *paths[] = { + "foo", "z", "y", "x", "foo", + "bar", "c", "b", "a", "bar", + NULL + }; + char **expect = paths; + FTS *fts; + FTSENT *ftse; + + ATF_REQUIRE_EQ(0, mkdir("bar", 0755)); + ATF_REQUIRE_EQ(0, close(creat("bar/a", 0644))); + ATF_REQUIRE_EQ(0, close(creat("bar/b", 0644))); + ATF_REQUIRE_EQ(0, close(creat("bar/c", 0644))); + ATF_REQUIRE_EQ(0, mkdir("foo", 0755)); + ATF_REQUIRE_EQ(0, close(creat("foo/x", 0644))); + ATF_REQUIRE_EQ(0, close(creat("foo/y", 0644))); + ATF_REQUIRE_EQ(0, close(creat("foo/z", 0644))); + fts = fts_open_b(args, 0, + ^(const FTSENT * const *a, const FTSENT * const *b) { + return (strcmp((*b)->fts_name, (*a)->fts_name)); + }); + ATF_REQUIRE_MSG(fts != NULL, "fts_open_b(): %m"); + while ((ftse = fts_read(fts)) != NULL && *expect != NULL) { + ATF_CHECK_STREQ(*expect, ftse->fts_name); + expect++; + } + ATF_CHECK_EQ(NULL, ftse); + ATF_CHECK_EQ(NULL, *expect); + ATF_REQUIRE_EQ_MSG(0, fts_close(fts), "fts_close(): %m"); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, fts_blocks_test); + return (atf_no_error()); +} diff --git a/lib/libc/tests/stdlib/Makefile b/lib/libc/tests/stdlib/Makefile index 15b8d3844d08..df44a42ac9b1 100644 --- a/lib/libc/tests/stdlib/Makefile +++ b/lib/libc/tests/stdlib/Makefile @@ -61,7 +61,7 @@ CFLAGS+= -I${.CURDIR} LIBADD.cxa_thread_atexit_test+= pthread -# Tests that requires Blocks feature +# Tests that require blocks support .for t in qsort_b_test CFLAGS.${t}.c+= -fblocks LIBADD.${t}+= BlocksRuntime