diff options
44 files changed, 795 insertions, 222 deletions
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9c3f8e..4075eb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,9 +5,11 @@ on: [push, pull_request] jobs: linux-x86: name: Linux (x86) - runs-on: ubuntu-24.04 + # Don't run on both pushes and pull requests + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 @@ -46,9 +48,10 @@ jobs: linux-arm: name: Linux (Arm64) - runs-on: ubuntu-24.04-arm + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 @@ -76,9 +79,10 @@ jobs: macos: name: macOS - runs-on: macos-15 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 @@ -93,14 +97,15 @@ jobs: freebsd: name: FreeBSD - runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 - name: Run tests - uses: cross-platform-actions/action@v0.27.0 + uses: cross-platform-actions/action@v0.28.0 with: operating_system: freebsd version: "14.2" @@ -120,17 +125,18 @@ jobs: openbsd: name: OpenBSD - runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 - name: Run tests - uses: cross-platform-actions/action@v0.27.0 + uses: cross-platform-actions/action@v0.28.0 with: operating_system: openbsd - version: "7.6" + version: "7.7" run: | sudo pkg_add \ @@ -148,14 +154,15 @@ jobs: netbsd: name: NetBSD - runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 - name: Run tests - uses: cross-platform-actions/action@v0.27.0 + uses: cross-platform-actions/action@v0.28.0 with: operating_system: netbsd version: "10.1" @@ -177,9 +184,10 @@ jobs: dragonflybsd: name: DragonFly BSD - runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 @@ -211,9 +219,10 @@ jobs: omnios: name: OmniOS - runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name != github.event.pull_request.base.repo.full_name + steps: - uses: actions/checkout@v4 @@ -92,7 +92,7 @@ OBJS += obj/src/main.o ${BINS}: @${MKDIR} ${@D} - +${MSG} "[ LD ] $@" ${CC} ${_CFLAGS} ${_LDFLAGS} ${.ALLSRC} ${_LDLIBS} -o $@ + +${MSG} "[ LD ] $@" ${CC} ${_CFLAGS} ${_LDFLAGS} $^ ${_LDLIBS} -o $@ ${POSTLINK} # Get the .c file for a .o file @@ -105,7 +105,7 @@ gen/version.i.new:: .SILENT: gen/version.i.new gen/version.i: gen/version.i.new - test -e $@ && cmp -s $@ ${.ALLSRC} && ${RM} ${.ALLSRC} || mv ${.ALLSRC} $@ + test -e $@ && cmp -s $@ $^ && ${RM} $^ || mv $^ $@ .SILENT: gen/version.i obj/src/version.o: gen/version.i diff --git a/bench/bench.sh b/bench/bench.sh index f249ffc..c9ed978 100644 --- a/bench/bench.sh +++ b/bench/bench.sh @@ -22,6 +22,7 @@ PRINT_DEFAULT=(linux) STRATEGIES_DEFAULT=(rust) JOBS_DEFAULT=(rust) EXEC_DEFAULT=(linux) +SORTED_DEFAULT=(chromium) usage() { printf 'Usage: tailfin run %s\n' "${BASH_SOURCE[0]}" @@ -60,6 +61,10 @@ usage() { printf ' Process spawning benchmark.\n' printf ' Default corpus is --exec=%s\n\n' "${EXEC_DEFAULT[*]}" + printf ' --sorted[=CORPUS]\n' + printf ' Sorted traversal benchmark.\n' + printf ' Default corpus is --sorted=%s\n\n' "${SORTED_DEFAULT[*]}" + printf ' --build=COMMIT\n' printf ' Build this bfs commit and benchmark it. Specify multiple times to\n' printf ' compare, e.g. --build=3.0.1 --build=3.0.2\n\n' @@ -121,6 +126,7 @@ setup() { STRATEGIES=() JOBS=() EXEC=() + SORTED=() for arg; do case "$arg" in @@ -195,6 +201,12 @@ setup() { --exec=*) read -ra EXEC <<<"${arg#*=}" ;; + --sorted) + SORTED=("${SORTED_DEFAULT[@]}") + ;; + --sorted=*) + read -ra SORTED <<<"${arg#*=}" + ;; --default) COMPLETE=("${COMPLETE_DEFAULT[@]}") EARLY_QUIT=("${EARLY_QUIT_DEFAULT[@]}") @@ -203,6 +215,7 @@ setup() { STRATEGIES=("${STRATEGIES_DEFAULT[@]}") JOBS=("${JOBS_DEFAULT[@]}") EXEC=("${EXEC_DEFAULT[@]}") + SORTED=("${SORTED_DEFAULT[@]}") ;; --help) usage @@ -227,7 +240,7 @@ setup() { as-user mkdir -p bench/corpus declare -A cloned=() - for corpus in "${COMPLETE[@]}" "${EARLY_QUIT[@]}" "${STAT[@]}" "${PRINT[@]}" "${STRATEGIES[@]}" "${JOBS[@]}" "${EXEC[@]}"; do + for corpus in "${COMPLETE[@]}" "${EARLY_QUIT[@]}" "${STAT[@]}" "${PRINT[@]}" "${STRATEGIES[@]}" "${JOBS[@]}" "${EXEC[@]}" "${SORTED[@]}"; do if ((cloned["$corpus"])); then continue fi @@ -283,6 +296,7 @@ setup() { export_array STRATEGIES export_array JOBS export_array EXEC + export_array SORTED if ((UID == 0)); then turbo-off @@ -650,6 +664,29 @@ bench-exec() { fi } +# Benchmark sorted traversal +bench-sorted-corpus() { + subgroup '%s' "$1" + + cmds=() + for bfs in "${BFS[@]}"; do + cmds+=("$bfs -s $2 -false") + done + + do-hyperfine "${cmds[@]}" +} + +# All sorted traversal benchmarks +bench-sorted() { + if (($#)); then + group "Sorted traversal" + + for corpus; do + bench-sorted-corpus "$corpus ${TAGS[$corpus]}" "bench/corpus/$corpus" + done + fi +} + # Print benchmarked versions bench-versions() { subgroup "Versions" @@ -698,6 +735,7 @@ bench() { import_array STRATEGIES import_array JOBS import_array EXEC + import_array SORTED bench-complete "${COMPLETE[@]}" bench-early-quit "${EARLY_QUIT[@]}" @@ -706,5 +744,6 @@ bench() { bench-strategies "${STRATEGIES[@]}" bench-jobs "${JOBS[@]}" bench-exec "${EXEC[@]}" + bench-sorted "${SORTED[@]}" bench-details } diff --git a/bench/ioq.c b/bench/ioq.c index 5db585a..fb9edbc 100644 --- a/bench/ioq.c +++ b/bench/ioq.c @@ -17,6 +17,149 @@ #include <time.h> #include <unistd.h> +/** A latency sample. */ +struct lat { + /** The sampled latency. */ + struct timespec time; + /** A random integer, for reservoir sampling. */ + long key; +}; + +/** Number of latency samples to keep. */ +#define SAMPLES 1000 +/** Latency sampling period. */ +#define PERIOD 128 + +/** Latency measurements. */ +struct lats { + /** Lowest observed latency. */ + struct timespec min; + /** Highest observed latency. */ + struct timespec max; + /** Total latency. */ + struct timespec sum; + /** Number of measured requests. */ + size_t count; + + /** Priority queue for reservoir sampling. */ + struct lat heap[SAMPLES]; + /** Current size of the heap. */ + size_t heap_size; +}; + +/** Initialize a latency reservoir. */ +static void lats_init(struct lats *lats) { + lats->min = (struct timespec) { .tv_sec = 1000 }; + lats->max = (struct timespec) { 0 }; + lats->sum = (struct timespec) { 0 }; + lats->count = 0; + lats->heap_size = 0; +} + +/** Binary heap parent. */ +static size_t heap_parent(size_t i) { + return (i - 1) / 2; +} + +/** Binary heap left child. */ +static size_t heap_child(size_t i) { + return 2 * i + 1; +} + +/** Binary heap smallest child. */ +static size_t heap_min_child(const struct lats *lats, size_t i) { + size_t j = heap_child(i); + size_t k = j + 1; + if (k < lats->heap_size && lats->heap[k].key < lats->heap[j].key) { + return k; + } else { + return j; + } +} + +/** Check if the heap property is met. */ +static bool heap_check(const struct lat *parent, const struct lat *child) { + return parent->key <= child->key; +} + +/** Reservoir sampling. */ +static void heap_push(struct lats *lats, const struct lat *lat) { + size_t i; + + if (lats->heap_size < SAMPLES) { + // Heapify up + i = lats->heap_size++; + while (i > 0) { + size_t j = heap_parent(i); + if (heap_check(&lats->heap[j], lat)) { + break; + } + lats->heap[i] = lats->heap[j]; + i = j; + } + } else if (lat->key > lats->heap[0].key) { + // Heapify down + i = 0; + while (true) { + size_t j = heap_min_child(lats, i); + if (j >= SAMPLES || heap_check(lat, &lats->heap[j])) { + break; + } + lats->heap[i] = lats->heap[j]; + i = j; + } + } else { + // Reject + return; + } + + lats->heap[i] = *lat; +} + +/** Add a latency sample. */ +static void lats_push(struct lats *lats, const struct timespec *ts) { + timespec_min(&lats->min, ts); + timespec_max(&lats->max, ts); + timespec_add(&lats->sum, ts); + ++lats->count; + + struct lat lat = { + .time = *ts, + .key = lrand48(), + }; + heap_push(lats, &lat); +} + +/** Merge two latency reservoirs. */ +static void lats_merge(struct lats *into, const struct lats *from) { + timespec_min(&into->min, &from->min); + timespec_max(&into->max, &from->max); + timespec_add(&into->sum, &from->sum); + into->count += from->count; + + for (size_t i = 0; i < from->heap_size; ++i) { + heap_push(into, &from->heap[i]); + } +} + +/** Latency qsort() comparator. */ +static int lat_cmp(const void *a, const void *b) { + const struct lat *la = a; + const struct lat *lb = b; + return timespec_cmp(&la->time, &lb->time); +} + +/** Sort the latency reservoir. */ +static void lats_sort(struct lats *lats) { + qsort(lats->heap, lats->heap_size, sizeof(lats->heap[0]), lat_cmp); +} + +/** Get the nth percentile. */ +static const struct timespec *lats_percentile(const struct lats *lats, int percent) { + size_t i = lats->heap_size * percent / 100; + return &lats->heap[i].time; +} + /** Which clock to use for benchmarking. */ static clockid_t clockid = CLOCK_REALTIME; @@ -38,86 +181,71 @@ struct times { /** Total requests finished. */ size_t popped; - /** Number of timed requests (latency). */ - size_t timed_reqs; /** The start time for the currently tracked request. */ struct timespec req_start; /** Whether a timed request is currently in flight. */ bool timing; /** Latency measurements. */ - struct { - struct timespec min; - struct timespec max; - struct timespec sum; - } latency; + struct lats lats; }; /** Initialize a timer. */ static void times_init(struct times *times) { - *times = (struct times) { - .latency = { - .min = { .tv_sec = 1000 }, - }, - }; gettime(×->start); -} - -/** Start timing a single request. */ -static void start_request(struct times *times) { - gettime(×->req_start); - times->timing = true; + times->pushed = 0; + times->popped = 0; + bfs_assert(!times->timing); + lats_init(×->lats); } /** Finish timing a request. */ -static void finish_request(struct times *times) { +static void track_latency(struct times *times) { struct timespec elapsed; gettime(&elapsed); timespec_sub(&elapsed, ×->req_start); - - timespec_min(×->latency.min, &elapsed); - timespec_max(×->latency.max, &elapsed); - timespec_add(×->latency.sum, &elapsed); + lats_push(×->lats, &elapsed); bfs_assert(times->timing); times->timing = false; - ++times->timed_reqs; } /** Add times to the totals, and reset the lap times. */ static void times_lap(struct times *total, struct times *lap) { total->pushed += lap->pushed; total->popped += lap->popped; - total->timed_reqs += lap->timed_reqs; - - timespec_min(&total->latency.min, &lap->latency.min); - timespec_max(&total->latency.max, &lap->latency.max); - timespec_add(&total->latency.sum, &lap->latency.sum); + lats_merge(&total->lats, &lap->lats); times_init(lap); } /** Print some times. */ -static void times_print(const struct times *times, long seconds) { +static void times_print(struct times *times, long seconds) { struct timespec elapsed; gettime(&elapsed); timespec_sub(&elapsed, ×->start); double fsec = timespec_ns(&elapsed) / 1.0e9; - double iops = times->popped / fsec; - double mean = timespec_ns(×->latency.sum) / times->timed_reqs; - double min = timespec_ns(×->latency.min); - double max = timespec_ns(×->latency.max); if (seconds > 0) { - printf("%9ld", seconds); + printf("%5ld", seconds); } else if (elapsed.tv_nsec >= 10 * 1000 * 1000) { - printf("%9.2f", fsec); + printf("%5.2f", fsec); } else { - printf("%9.0f", fsec); + printf("%5.0f", fsec); } - printf(" │ %'17.0f │ %'15.0f ∈ [%'6.0f .. %'7.0f]\n", iops, mean, min, max); + double iops = times->popped / fsec; + double mean = timespec_ns(×->lats.sum) / times->lats.count; + double min = timespec_ns(×->lats.min); + double max = timespec_ns(×->lats.max); + + lats_sort(×->lats); + double n50 = timespec_ns(lats_percentile(×->lats, 50)); + double n90 = timespec_ns(lats_percentile(×->lats, 90)); + double n99 = timespec_ns(lats_percentile(×->lats, 99)); + + printf(" │ %'12.0f │ %'7.0f │ %'7.0f │ %'7.0f │ %'7.0f │ %'7.0f │ %'7.0f\n", iops, mean, min, n50, n90, n99, max); fflush(stdout); } @@ -126,9 +254,9 @@ static bool push(struct ioq *ioq, enum ioq_nop_type type, struct times *lap) { void *ptr = NULL; // Track latency for a small fraction of requests - if (!lap->timing && (lap->pushed + 1) % 128 == 0) { - start_request(lap); + if (!lap->timing && (lap->pushed + 1) % PERIOD == 0) { ptr = lap; + gettime(&lap->req_start); } int ret = ioq_nop(ioq, type, ptr); @@ -138,6 +266,9 @@ static bool push(struct ioq *ioq, enum ioq_nop_type type, struct times *lap) { } ++lap->pushed; + if (ptr) { + lap->timing = true; + } return true; } @@ -149,7 +280,7 @@ static bool pop(struct ioq *ioq, struct times *lap, bool block) { } if (ent->ptr) { - finish_request(lap); + track_latency(lap); } ioq_free(ioq, ent); @@ -177,9 +308,9 @@ int main(int argc, char *argv[]) { setlocale(LC_ALL, ""); // -d: queue depth - long depth = 4096; + unsigned int depth = 4096; // -j: threads - long threads = 0; + unsigned int threads = 0; // -t: timeout double timeout = 5.0; // -L|-H: ioq_nop() type @@ -190,13 +321,13 @@ int main(int argc, char *argv[]) { while (c = getopt(argc, argv, ":d:j:t:LH"), c != -1) { switch (c) { case 'd': - if (xstrtol(optarg, NULL, 10, &depth) != 0) { + if (xstrtoui(optarg, NULL, 10, &depth) != 0) { fprintf(stderr, "%s: Bad depth '%s': %s\n", cmd, optarg, errstr()); return EXIT_FAILURE; } break; case 'j': - if (xstrtol(optarg, NULL, 10, &threads) != 0) { + if (xstrtoui(optarg, NULL, 10, &threads) != 0) { fprintf(stderr, "%s: Bad thread count '%s': %s\n", cmd, optarg, errstr()); return EXIT_FAILURE; } @@ -222,7 +353,7 @@ int main(int argc, char *argv[]) { } } - if (threads <= 0) { + if (!threads) { threads = nproc(); if (threads > 8) { threads = 8; @@ -238,8 +369,8 @@ int main(int argc, char *argv[]) { printf("I/O queue benchmark (%s)\n\n", bfs_version); - printf("[-d] depth: %ld\n", depth); - printf("[-j] threads: %ld (including main)\n", threads + 1); + printf("[-d] depth: %u\n", depth); + printf("[-j] threads: %u (including main)\n", threads + 1); if (type == IOQ_NOP_HEAVY) { printf("[-H] type: heavy (with syscalls)\n"); } else { @@ -247,12 +378,13 @@ int main(int argc, char *argv[]) { } printf("\n"); - printf(" Time (s) │ Throughput (IO/s) │ Latency (ns/IO)\n"); - printf("══════════╪═══════════════════╪═════════════════\n"); + printf(" Time │ Throughput │ Latency │ min │ 50%% │ 90%% │ 99%% │ max\n"); + printf(" (s) │ (IO/s) │ (ns/IO) │ │ │ │ │\n"); + printf("══════╪══════════════╪═════════╪═════════╪═════════╪═════════╪═════════╪═════════\n"); fflush(stdout); struct ioq *ioq = ioq_create(depth, threads); - bfs_everify(ioq, "ioq_create(%ld, %ld)", depth, threads); + bfs_everify(ioq, "ioq_create(%u, %u)", depth, threads); // Pre-allocate all the requests while (ioq_capacity(ioq) > 0) { @@ -311,9 +443,9 @@ int main(int argc, char *argv[]) { times_lap(&total, &lap); if (load(&quit, relaxed)) { - printf("\r────^C────┼───────────────────┼─────────────────\n"); + printf("\r──^C──┼──────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────\n"); } else { - printf("──────────┼───────────────────┼─────────────────\n"); + printf("──────┼──────────────┼─────────┼─────────┼─────────┼─────────┼─────────┼─────────\n"); } times_print(&total, 0); diff --git a/build/config.mk b/build/config.mk index 6296168..663926c 100644 --- a/build/config.mk +++ b/build/config.mk @@ -7,21 +7,15 @@ include build/prelude.mk include build/exports.mk # All configuration steps -config: gen/config.mk +config: gen/config.mk gen/config.h .PHONY: config -# Makefile fragments generated by `./configure` -MKS := \ - gen/vars.mk \ - gen/flags.mk \ - gen/pkgs.mk - # The main configuration file, which includes the others -gen/config.mk: ${MKS} gen/config.h +gen/config.mk: gen/vars.mk gen/flags.mk gen/pkgs.mk ${MSG} "[ GEN] $@" @printf '# %s\n' "$@" >$@ - @printf 'include %s\n' ${MKS} >>$@ - ${VCAT} gen/config.mk + @printf 'include %s\n' $^ >>$@ + ${VCAT} $@ .PHONY: gen/config.mk # Saves the configurable variables diff --git a/build/flags.mk b/build/flags.mk index fbdf8cf..3748a8a 100644 --- a/build/flags.mk +++ b/build/flags.mk @@ -110,8 +110,8 @@ gen/flags.mk: ${AUTO_FLAGS} @printf '_LDLIBS := %s\n' "$$XLDLIBS" >>$@ @printf 'NOLIBS := %s\n' "$$XNOLIBS" >>$@ @test "${OS}-${SAN}" != FreeBSD-y || printf 'POSTLINK = elfctl -e +noaslr $$@\n' >>$@ - @cat ${.ALLSRC} >>$@ - @cat ${.ALLSRC:%=%.log} >gen/flags.log + @cat $^ >>$@ + @cat ${^:%=%.log} >gen/flags.log ${VCAT} $@ .PHONY: gen/flags.mk diff --git a/build/header.mk b/build/header.mk index 13672ba..f15829a 100644 --- a/build/header.mk +++ b/build/header.mk @@ -70,9 +70,9 @@ gen/config.h: ${PKG_HEADERS} ${HEADERS} @printf '// %s\n' "$@" >$@ @printf '#ifndef BFS_CONFIG_H\n' >>$@ @printf '#define BFS_CONFIG_H\n' >>$@ - @cat ${.ALLSRC} >>$@ + @cat $^ >>$@ @printf '#endif // BFS_CONFIG_H\n' >>$@ - @cat gen/flags.log ${.ALLSRC:%=%.log} >gen/config.log + @cat gen/flags.log ${^:%=%.log} >gen/config.log ${VCAT} $@ @printf '%s' "$$CONFFLAGS" | build/embed.sh >gen/confflags.i @printf '%s' "$$XCC" | build/embed.sh >gen/cc.i diff --git a/build/pkgs.mk b/build/pkgs.mk index 5de9ac2..f692739 100644 --- a/build/pkgs.mk +++ b/build/pkgs.mk @@ -19,7 +19,7 @@ gen/pkgs.mk: ${HEADERS} printf '_LDFLAGS += %s\n' "$$(build/pkgconf.sh --ldflags "$$@")"; \ printf '_LDLIBS := %s $${_LDLIBS}\n' "$$(build/pkgconf.sh --ldlibs "$$@")"; \ }; \ - gen $$(grep -l ' true$$' ${.ALLSRC} | sed 's|.*/\(.*\)\.h|\1|') >>$@ + gen $$(grep -l ' true$$' $^ | sed 's|.*/\(.*\)\.h|\1|') >>$@ ${VCAT} $@ .PHONY: gen/pkgs.mk diff --git a/build/prelude.mk b/build/prelude.mk index c25dea4..6250d73 100644 --- a/build/prelude.mk +++ b/build/prelude.mk @@ -9,11 +9,9 @@ # We don't use any suffix rules .SUFFIXES: -# GNU make has $^ for the full list of targets, while BSD make has $> and the -# long-form ${.ALLSRC}. We could write $^ $> to get them both, but that would -# break if one of them implemented support for the other. So instead, bring -# BSD's ${.ALLSRC} to GNU. -.ALLSRC ?= $^ +# GNU make has $^ for the full list of targets, while BSD make has $> (and the +# long-form ${.ALLSRC}). We use the GNU version, bringing it to BSD like this: +^ ?= $> # Installation paths DESTDIR ?= diff --git a/build/version.sh b/build/version.sh index 8f89921..ec0663a 100755 --- a/build/version.sh +++ b/build/version.sh @@ -14,5 +14,5 @@ if [ "${VERSION-}" ]; then elif [ -e "$DIR/.git" ] && command -v git >/dev/null 2>&1; then git -C "$DIR" describe --always --dirty else - echo "4.0.6" + echo "4.0.8" fi @@ -16,7 +16,7 @@ help() { Usage: \$ $0 [--enable-*|--disable-*] [--with-*|--without-*] [CC=...] [...] - \$ $MAKE -j$(nproc) + \$ $MAKE -j$(_nproc) Variables set in the environment or on the command line will be picked up: @@ -66,7 +66,7 @@ Packaging: This script is a thin wrapper around a makefile-based configuration system. Any other arguments will be passed directly to the $MAKE invocation, e.g. - \$ $0 -j$(nproc) V=1 + \$ $0 -j$(_nproc) V=1 EOF } @@ -85,11 +85,9 @@ invalid() { } # Get the number of cores to use -nproc() { +_nproc() { { - # Run command nproc in a subshell to work around a bash 3 bug - # https://stackoverflow.com/q/68143965 - (command nproc) \ + nproc \ || sysctl -n hw.ncpu \ || getconf _NPROCESSORS_ONLN \ || echo 1 @@ -233,7 +231,7 @@ for f in Makefile bench build completions docs src tests; do test -e "$f" || ln -s "$DIR/$f" "$f" done -# Set MAKEFLAGS to -j$(nproc) if it's unset -export MAKEFLAGS="${MAKEFLAGS--j$(nproc)}" +# Set MAKEFLAGS to -j$(_nproc) if it's unset +export MAKEFLAGS="${MAKEFLAGS--j$(_nproc)}" $MAKE -rf build/config.mk "$@" diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3e72baf..56f53b4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,50 @@ 4.* === +4.0.8 +----- + +**June 20, 2025** + +### Bug fixes + +- Fixed an invalid optimization that transformed + + $ bfs -user you -or -user me + + into just + + $ bfs -user you + + The bug was originally introduced in bfs 2.0 (October 14, 2020). + ([#155](https://github.com/tavianator/bfs/issues/155)) + + +4.0.7 +----- + +**June 15, 2025** + +### Changes + +- `bfs` now takes CPU affinity into account when picking how many threads to use + ([`a36774b`](https://github.com/tavianator/bfs/commit/a36774be636c3429c6e73de33bf65a1bdbdcfb4b)) + +- `-execdir /bin/...` is now allowed even with a relative path in `$PATH` + ([`cb40f51`](https://github.com/tavianator/bfs/commit/cb40f51e4e6375a10265484b6959c6b1b0591378)) + +- *Expect* is no longer a test suite dependency + ([`7102fec`](https://github.com/tavianator/bfs/commit/7102fec257835302cb4978160bba4cbebd0b63e1)) + +### Bug fixes + +- Only the last `-files0-from` argument now has any effect, to match GNU find + ([`a662fda`](https://github.com/tavianator/bfs/commit/a662fda2642e17478bc8e78adb4c6642a8505cdb)) + +- Fixed `-execdir {}`, which was inadvertently broken in bfs 4.0 + ([`def4a83`](https://github.com/tavianator/bfs/commit/def4a832425bfe94b96b8cb1146a83552b754fb4)) + + 4.0.6 ----- @@ -1,6 +1,6 @@ .\" Copyright © Tavian Barnes <tavianator@tavianator.com> .\" SPDX-License-Identifier: 0BSD -.TH BFS 1 2025-02-26 "bfs 4.0.6" +.TH BFS 1 2025-06-15 "bfs 4.0.8" .SH NAME bfs \- breadth-first search for your files .SH SYNOPSIS @@ -219,6 +219,15 @@ extern const char bfs_ldlibs[]; #endif /** + * Mark the size of a flexible array member. + */ +#if __has_attribute(counted_by) +# define _counted_by(...) __attribute__((counted_by(__VA_ARGS__))) +#else +# define _counted_by(...) +#endif + +/** * Optimization hint to not unroll a loop. */ #if BFS_HAS_PRAGMA_NOUNROLL diff --git a/src/bfstd.c b/src/bfstd.c index 4269d55..b78af7a 100644 --- a/src/bfstd.c +++ b/src/bfstd.c @@ -235,6 +235,36 @@ static int xstrtox_epilogue(const char *str, char **end, char *endp) { return 0; } +int xstrtos(const char *str, char **end, int base, short *value) { + long n; + if (xstrtol(str, end, base, &n) != 0) { + return -1; + } + + if (n < SHRT_MIN || n > SHRT_MAX) { + errno = ERANGE; + return -1; + } + + *value = n; + return 0; +} + +int xstrtoi(const char *str, char **end, int base, int *value) { + long n; + if (xstrtol(str, end, base, &n) != 0) { + return -1; + } + + if (n < INT_MIN || n > INT_MAX) { + errno = ERANGE; + return -1; + } + + *value = n; + return 0; +} + int xstrtol(const char *str, char **end, int base, long *value) { if (xstrtox_prologue(str) != 0) { return -1; @@ -275,6 +305,70 @@ int xstrtod(const char *str, char **end, double *value) { return xstrtox_epilogue(str, end, endp); } +int xstrtous(const char *str, char **end, int base, unsigned short *value) { + unsigned long n; + if (xstrtoul(str, end, base, &n) != 0) { + return -1; + } + + if (n > USHRT_MAX) { + errno = ERANGE; + return -1; + } + + *value = n; + return 0; +} + +int xstrtoui(const char *str, char **end, int base, unsigned int *value) { + unsigned long n; + if (xstrtoul(str, end, base, &n) != 0) { + return -1; + } + + if (n > UINT_MAX) { + errno = ERANGE; + return -1; + } + + *value = n; + return 0; +} + +/** Common epilogue for xstrtou*() wrappers. */ +static int xstrtoux_epilogue(const char *str, char **end, char *endp) { + if (xstrtox_epilogue(str, end, endp) != 0) { + return -1; + } + + if (str[0] == '-') { + errno = ERANGE; + return -1; + } + + return 0; +} + +int xstrtoul(const char *str, char **end, int base, unsigned long *value) { + if (xstrtox_prologue(str) != 0) { + return -1; + } + + char *endp; + *value = strtoul(str, &endp, base); + return xstrtoux_epilogue(str, end, endp); +} + +int xstrtoull(const char *str, char **end, int base, unsigned long long *value) { + if (xstrtox_prologue(str) != 0) { + return -1; + } + + char *endp; + *value = strtoull(str, &endp, base); + return xstrtoux_epilogue(str, end, endp); +} + /** Compile and execute a regular expression for xrpmatch(). */ static int xrpregex(nl_item item, const char *response) { const char *pattern = nl_langinfo(item); diff --git a/src/bfstd.h b/src/bfstd.h index 51920c9..15dd949 100644 --- a/src/bfstd.h +++ b/src/bfstd.h @@ -169,6 +169,16 @@ char *xgetdelim(FILE *file, char delim); const char *xgetprogname(void); /** + * Like xstrtol(), but for short. + */ +int xstrtos(const char *str, char **end, int base, short *value); + +/** + * Like xstrtol(), but for int. + */ +int xstrtoi(const char *str, char **end, int base, int *value); + +/** * Wrapper for strtol() that forbids leading spaces. */ int xstrtol(const char *str, char **end, int base, long *value); @@ -179,6 +189,26 @@ int xstrtol(const char *str, char **end, int base, long *value); int xstrtoll(const char *str, char **end, int base, long long *value); /** + * Like xstrtoul(), but for unsigned short. + */ +int xstrtous(const char *str, char **end, int base, unsigned short *value); + +/** + * Like xstrtoul(), but for unsigned int. + */ +int xstrtoui(const char *str, char **end, int base, unsigned int *value); + +/** + * Wrapper for strtoul() that forbids leading spaces, negatives. + */ +int xstrtoul(const char *str, char **end, int base, unsigned long *value); + +/** + * Wrapper for strtoull() that forbids leading spaces, negatives. + */ +int xstrtoull(const char *str, char **end, int base, unsigned long long *value); + +/** * Wrapper for strtof() that forbids leading spaces. */ int xstrtof(const char *str, char **end, float *value); @@ -253,7 +253,7 @@ struct bftw_file { /** The length of the file's name. */ size_t namelen; /** The file's name. */ - char name[]; + char name[]; // _counted_by(namelen + 1) }; /** @@ -1485,7 +1485,8 @@ fail: /** Check if we should stat() a file asynchronously. */ static bool bftw_should_ioq_stat(struct bftw_state *state, struct bftw_file *file) { - // To avoid surprising users too much, process the roots in order + // POSIX wants the root paths to be processed in order + // See https://www.austingroupbugs.net/view.php?id=1859 if (file->depth == 0) { return false; } diff --git a/src/color.c b/src/color.c index 588dbac..a026831 100644 --- a/src/color.c +++ b/src/color.c @@ -32,7 +32,7 @@ struct esc_seq { /** The length of the escape sequence. */ size_t len; /** The escape sequence itself, without a terminating NUL. */ - char seq[]; + char seq[] _counted_by(len); }; /** @@ -48,7 +48,7 @@ struct ext_color { /** Whether the comparison should be case-sensitive. */ bool case_sensitive; /** The extension to match (NUL-terminated). */ - char ext[]; + char ext[]; // _counted_by(len + 1); }; struct colors { diff --git a/src/dstring.c b/src/dstring.c index 0f08679..678d685 100644 --- a/src/dstring.c +++ b/src/dstring.c @@ -23,7 +23,7 @@ struct dstring { /** Length of the string, *excluding* the terminating NUL. */ size_t len; /** The string itself. */ - alignas(dchar) char str[]; + alignas(dchar) char str[] _counted_by(cap); }; #define DSTR_OFFSET offsetof(struct dstring, str) @@ -203,7 +203,7 @@ struct ioqq { cache_align atomic size_t tail; /** The circular buffer itself. */ - cache_align ioq_slot slots[]; + cache_align ioq_slot slots[]; // _counted_by(slot_mask + 1) }; /** Destroy an I/O command queue. */ @@ -593,7 +593,7 @@ struct ioq { /** The number of background threads. */ size_t nthreads; /** The background threads themselves. */ - struct ioq_thread threads[]; + struct ioq_thread threads[] _counted_by(nthreads); }; /** Cancel a request if we need to. */ @@ -1623,14 +1623,19 @@ static void data_flow_icmp(struct bfs_opt *opt, const struct bfs_expr *expr, enu /** Transfer function for -{execut,read,writ}able. */ static struct bfs_expr *data_flow_access(struct bfs_opt *opt, struct bfs_expr *expr, const struct visitor *visitor) { - if (expr->num & R_OK) { + switch (expr->num) { + case R_OK: data_flow_pred(opt, READABLE_PRED, true); - } - if (expr->num & W_OK) { + break; + case W_OK: data_flow_pred(opt, WRITABLE_PRED, true); - } - if (expr->num & X_OK) { + break; + case X_OK: data_flow_pred(opt, EXECUTABLE_PRED, true); + break; + default: + bfs_bug("Unknown access() mode %lld", expr->num); + break; } return expr; @@ -1655,7 +1660,7 @@ static struct bfs_expr *data_flow_gid(struct bfs_opt *opt, struct bfs_expr *expr gid_t gid = range->min; bool nogroup = !bfs_getgrgid(opt->ctx->groups, gid); if (errno == 0) { - data_flow_pred(opt, NOGROUP_PRED, nogroup); + constrain_pred(&opt->after_true.preds[NOGROUP_PRED], nogroup); } } @@ -1729,7 +1734,7 @@ static struct bfs_expr *data_flow_uid(struct bfs_opt *opt, struct bfs_expr *expr uid_t uid = range->min; bool nouser = !bfs_getpwuid(opt->ctx->users, uid); if (errno == 0) { - data_flow_pred(opt, NOUSER_PRED, nouser); + constrain_pred(&opt->after_true.preds[NOUSER_PRED], nouser); } } diff --git a/src/parse.c b/src/parse.c index 9c39d6b..5ec4c0e 100644 --- a/src/parse.c +++ b/src/parse.c @@ -1247,6 +1247,41 @@ static struct bfs_expr *parse_empty(struct bfs_parser *parser, int arg1, int arg return expr; } +/** Check for unsafe relative paths in $PATH. */ +static const char *unsafe_path(const struct bfs_exec *execbuf) { + if (!(execbuf->flags & BFS_EXEC_CHDIR)) { + // Not -execdir or -okdir + return NULL; + } + + const char *exe = execbuf->tmpl_argv[0]; + if (strchr(exe, '/')) { + // No $PATH lookups for /foo or foo/bar + return NULL; + } + + if (strstr(exe, "{}")) { + // Substituted paths always contain a / + return NULL; + } + + const char *path = getenv("PATH"); + while (path) { + if (path[0] != '/') { + // Relative $PATH component! + return path; + } + + path = strchr(path, ':'); + if (path) { + ++path; + } + } + + // No relative components in $PATH + return NULL; +} + /** * Parse -exec(dir)?/-ok(dir)?. */ @@ -1269,29 +1304,21 @@ static struct bfs_expr *parse_exec(struct bfs_parser *parser, int flags, int arg // For pipe() in bfs_spawn() expr->ephemeral_fds = 2; - if (execbuf->flags & BFS_EXEC_CHDIR) { - // Check for relative paths in $PATH - const char *path = getenv("PATH"); - while (path) { - if (*path != '/') { - size_t len = strcspn(path, ":"); - char *comp = strndup(path, len); - if (comp) { - parse_expr_error(parser, expr, - "This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp); - free(comp); - } else { - parse_perror(parser, "strndup()"); - } - return NULL; - } - - path = strchr(path, ':'); - if (path) { - ++path; - } + const char *unsafe = unsafe_path(execbuf); + if (unsafe) { + size_t len = strcspn(unsafe, ":"); + char *comp = strndup(unsafe, len); + if (comp) { + parse_expr_error(parser, expr, + "This action would be unsafe, since ${bld}$$PATH${rs} contains the relative path ${bld}%pq${rs}\n", comp); + free(comp); + } else { + parse_perror(parser, "strndup()"); } + return NULL; + } + if (execbuf->flags & BFS_EXEC_CHDIR) { // To dup() the parent directory if (execbuf->flags & BFS_EXEC_MULTI) { ++expr->persistent_fds; @@ -129,7 +129,7 @@ struct trie_node { * tag to distinguish internal nodes from leaves. This is safe as long * as all dynamic allocations are aligned to more than a single byte. */ - uintptr_t children[]; + uintptr_t children[]; // _counted_by(count_ones(bitmap)) }; /** Check if an encoded pointer is to an internal node. */ @@ -21,7 +21,7 @@ struct trie_leaf { /** The length of the key in bytes. */ size_t length; /** The key itself, stored inline. */ - char key[]; + char key[] _counted_by(length); }; /** diff --git a/src/xspawn.c b/src/xspawn.c index 3fa4e60..ee62c05 100644 --- a/src/xspawn.c +++ b/src/xspawn.c @@ -232,12 +232,28 @@ int bfs_spawn_adddup2(struct bfs_spawn *ctx, int oldfd, int newfd) { */ #define BFS_POSIX_SPAWNP_AFTER_FCHDIR !(__APPLE__ || __NetBSD__) +/** + * NetBSD even resolves the executable before file actions with posix_spawn()! + */ +#define BFS_POSIX_SPAWN_AFTER_FCHDIR !__NetBSD__ + int bfs_spawn_addfchdir(struct bfs_spawn *ctx, int fd) { struct bfs_spawn_action *action = bfs_spawn_action(BFS_SPAWN_FCHDIR); if (!action) { return -1; } +#if __APPLE__ + // macOS has a bug that causes EBADF when an fchdir() action refers to a + // file opened by the file actions + for_slist (struct bfs_spawn_action, prev, ctx) { + if (fd == prev->out_fd) { + bfs_spawn_clear_posix(ctx); + break; + } + } +#endif + #if BFS_HAS_POSIX_SPAWN_ADDFCHDIR # define BFS_POSIX_SPAWN_ADDFCHDIR posix_spawn_file_actions_addfchdir #elif BFS_HAS_POSIX_SPAWN_ADDFCHDIR_NP @@ -401,18 +417,40 @@ static bool bfs_resolve_relative(const struct bfs_resolver *res) { return false; } +/** Check if the actions include fchdir(). */ +static bool bfs_spawn_will_chdir(const struct bfs_spawn *ctx) { + if (ctx) { + for_slist (const struct bfs_spawn_action, action, ctx) { + if (action->op == BFS_SPAWN_FCHDIR) { + return true; + } + } + } + + return false; +} + +/** Check if we can call xfaccessat() before file actions. */ +static bool bfs_can_access_early(const struct bfs_resolver *res, const struct bfs_spawn *ctx) { + if (res->exe[0] == '/') { + return true; + } + + if (bfs_spawn_will_chdir(ctx)) { + return false; + } + + return true; +} + /** Check if we can resolve the executable before file actions. */ static bool bfs_can_resolve_early(const struct bfs_resolver *res, const struct bfs_spawn *ctx) { if (!bfs_resolve_relative(res)) { return true; } - if (ctx) { - for_slist (const struct bfs_spawn_action, action, ctx) { - if (action->op == BFS_SPAWN_FCHDIR) { - return false; - } - } + if (bfs_spawn_will_chdir(ctx)) { + return false; } return true; @@ -442,17 +480,19 @@ static int bfs_resolve_early(struct bfs_resolver *res, const char *exe, const st }; if (bfs_can_skip_resolve(res, ctx)) { - // Do this check eagerly, even though posix_spawn()/execv() also - // would, because: - // - // - faccessat() is faster than fork()/clone() + execv() - // - posix_spawn() is not guaranteed to report ENOENT - if (xfaccessat(AT_FDCWD, exe, X_OK) == 0) { - res->done = true; - return 0; - } else { - return -1; + if (bfs_can_access_early(res, ctx)) { + // Do this check eagerly, even though posix_spawn()/execv() also + // would, because: + // + // - faccessat() is faster than fork()/clone() + execv() + // - posix_spawn() is not guaranteed to report ENOENT + if (xfaccessat(AT_FDCWD, exe, X_OK) != 0) { + return -1; + } } + + res->done = true; + return 0; } res->path = getenv("PATH"); @@ -529,6 +569,12 @@ static bool bfs_use_posix_spawn(const struct bfs_resolver *res, const struct bfs } #endif +#if !BFS_POSIX_SPAWN_AFTER_FCHDIR + if (res->exe[0] != '/' && bfs_spawn_will_chdir(ctx)) { + return false; + } +#endif + return true; } diff --git a/tests/bfs/execdir_path_relative_slash.out b/tests/bfs/execdir_path_relative_slash.out new file mode 100644 index 0000000..62b31f6 --- /dev/null +++ b/tests/bfs/execdir_path_relative_slash.out @@ -0,0 +1,19 @@ +./a +./b +./bar +./bar +./basic +./baz +./c +./d +./e +./f +./foo +./foo +./foo +./g +./h +./i +./j +./k +./l diff --git a/tests/bfs/execdir_path_relative_slash.sh b/tests/bfs/execdir_path_relative_slash.sh new file mode 100644 index 0000000..fb5a924 --- /dev/null +++ b/tests/bfs/execdir_path_relative_slash.sh @@ -0,0 +1 @@ +PATH="foo:$PATH" bfs_diff basic -execdir /bin/sh -c 'printf "%s\\n" "$@"' sh {} + diff --git a/tests/bfstd.c b/tests/bfstd.c index 685aa6c..6e15e2b 100644 --- a/tests/bfstd.c +++ b/tests/bfstd.c @@ -4,7 +4,6 @@ #include "tests.h" #include "bfstd.h" -#include "bit.h" #include "diag.h" #include <errno.h> @@ -95,37 +94,74 @@ static void check_wordescs(void) { /** xstrto*() test cases. */ static void check_strtox(void) { + short s; + unsigned short us; + int i; + unsigned int ui; long l; + unsigned long ul; long long ll; + unsigned long long ull; char *end; +#define check_strtouerr(err, str, end, base) \ + do { \ + bfs_echeck(xstrtous(str, end, base, &us) != 0 && errno == err); \ + bfs_echeck(xstrtoui(str, end, base, &ui) != 0 && errno == err); \ + bfs_echeck(xstrtoul(str, end, base, &ul) != 0 && errno == err); \ + bfs_echeck(xstrtoull(str, end, base, &ull) != 0 && errno == err); \ + } while (0) + + check_strtouerr(ERANGE, "-1", NULL, 0); + check_strtouerr(ERANGE, "-0x1", NULL, 0); + + check_strtouerr(EINVAL, "-", NULL, 0); + check_strtouerr(EINVAL, "-q", NULL, 0); + check_strtouerr(EINVAL, "-1q", NULL, 0); + check_strtouerr(EINVAL, "-0x", NULL, 0); + #define check_strtoerr(err, str, end, base) \ - bfs_echeck(xstrtol(str, end, base, &l) != 0 && errno == err); \ - bfs_echeck(xstrtoll(str, end, base, &ll) != 0 && errno == err) + do { \ + bfs_echeck(xstrtos(str, end, base, &s) != 0 && errno == err); \ + bfs_echeck(xstrtoi(str, end, base, &i) != 0 && errno == err); \ + bfs_echeck(xstrtol(str, end, base, &l) != 0 && errno == err); \ + bfs_echeck(xstrtoll(str, end, base, &ll) != 0 && errno == err); \ + check_strtouerr(err, str, end, base); \ + } while (0) check_strtoerr(EINVAL, "", NULL, 0); check_strtoerr(EINVAL, "", &end, 0); check_strtoerr(EINVAL, " 1 ", &end, 0); + check_strtoerr(EINVAL, " -1", NULL, 0); check_strtoerr(EINVAL, " 123", NULL, 0); check_strtoerr(EINVAL, "123 ", NULL, 0); check_strtoerr(EINVAL, "0789", NULL, 0); check_strtoerr(EINVAL, "789A", NULL, 0); check_strtoerr(EINVAL, "0x", NULL, 0); check_strtoerr(EINVAL, "0x789A", NULL, 10); - - if (LLONG_WIDTH == 64) { - check_strtoerr(ERANGE, "9223372036854775808", NULL, 0); - } + check_strtoerr(EINVAL, "0x-1", NULL, 0); + +#define check_strtotype(type, min, max, fmt, fn, str, base, v, n) \ + do { \ + if ((n) >= min && (n) <= max) { \ + bfs_echeck(fn(str, NULL, base, &v) == 0); \ + bfs_check(v == (type)(n), "%s('%s') == " fmt " (!= " fmt ")", #fn, str, v, (type)(n)); \ + } else { \ + bfs_echeck(fn(str, NULL, base, &v) != 0 && errno == ERANGE); \ + } \ + } while (0) #define check_strtoint(str, base, n) \ - if ((n) >= LONG_MIN && (n) <= LONG_MAX) { \ - bfs_echeck(xstrtol(str, NULL, base, &l) == 0); \ - bfs_check(l == (n), "xstrtol('%s') == %ld (!= %ld)", str, l, (long)(n)); \ - } else { \ - bfs_echeck(xstrtol(str, NULL, base, &l) != 0 && errno == ERANGE); \ - } \ - bfs_echeck(xstrtoll(str, NULL, base, &ll) == 0); \ - bfs_check(ll == (n), "xstrtoll('%s') == %lld (!= %lld)", str, ll, (long long)(n)) \ + do { \ + check_strtotype( signed short, SHRT_MIN, SHRT_MAX, "%d", xstrtos, str, base, s, n); \ + check_strtotype( signed int, INT_MIN, INT_MAX, "%d", xstrtoi, str, base, i, n); \ + check_strtotype( signed long, LONG_MIN, LONG_MAX, "%ld", xstrtol, str, base, l, n); \ + check_strtotype( signed long long, LLONG_MIN, LLONG_MAX, "%lld", xstrtoll, str, base, ll, n); \ + check_strtotype(unsigned short, 0, USHRT_MAX, "%u", xstrtous, str, base, us, n); \ + check_strtotype(unsigned int, 0, UINT_MAX, "%u", xstrtoui, str, base, ui, n); \ + check_strtotype(unsigned long, 0, ULONG_MAX, "%lu", xstrtoul, str, base, ul, n); \ + check_strtotype(unsigned long long, 0, ULLONG_MAX, "%llu", xstrtoull, str, base, ull, n); \ + } while (0) check_strtoint("123", 0, 123); check_strtoint("+123", 0, 123); @@ -139,13 +175,21 @@ static void check_strtox(void) { check_strtoint("123", 16, 0x123); - check_strtoint("9223372036854775807", 0, 9223372036854775807LL); - check_strtoint("-9223372036854775808", 0, -9223372036854775807LL - 1); + check_strtoint("0x7FFF", 0, 0x7FFF); + check_strtoint("-0x8000", 0, -0x8000); + + check_strtoint("0x7FFFFFFF", 0, 0x7FFFFFFFL); + check_strtoint("-0x80000000", 0, -0x7FFFFFFFL - 1); + + check_strtoint("0x7FFFFFFFFFFFFFFF", 0, 0x7FFFFFFFFFFFFFFFLL); + check_strtoint("-0x8000000000000000", 0, -0x7FFFFFFFFFFFFFFFLL - 1); #define check_strtoend(str, estr, base, n) \ - bfs_echeck(xstrtoll(str, &end, base, &ll) == 0); \ - bfs_check(ll == (n), "xstrtoll('%s') == %lld (!= %lld)", str, ll, (long long)(n)); \ - bfs_check(strcmp(end, estr) == 0, "xstrtoll('%s'): end == '%s' (!= '%s')", str, end, estr) \ + do { \ + bfs_echeck(xstrtoll(str, &end, base, &ll) == 0); \ + bfs_check(ll == (n), "xstrtoll('%s') == %lld (!= %lld)", str, ll, (long long)(n)); \ + bfs_check(strcmp(end, estr) == 0, "xstrtoll('%s'): end == '%s' (!= '%s')", str, end, estr); \ + } while (0) check_strtoend("123 ", " ", 0, 123); check_strtoend("0789", "89", 0, 07); diff --git a/tests/getopts.sh b/tests/getopts.sh index 255f2fa..a16511f 100644 --- a/tests/getopts.sh +++ b/tests/getopts.sh @@ -5,11 +5,7 @@ ## Argument parsing -if command -v nproc &>/dev/null; then - JOBS=$(nproc) -else - JOBS=1 -fi +JOBS=$(_nproc) MAKE= PATTERNS=() SUDO=() diff --git a/tests/gnu/execdir_self.out b/tests/gnu/execdir_self.out new file mode 100644 index 0000000..3ad0640 --- /dev/null +++ b/tests/gnu/execdir_self.out @@ -0,0 +1 @@ +./bar.sh diff --git a/tests/gnu/execdir_self.sh b/tests/gnu/execdir_self.sh new file mode 100644 index 0000000..1fc5d04 --- /dev/null +++ b/tests/gnu/execdir_self.sh @@ -0,0 +1,9 @@ +cd "$TEST" +mkdir foo +cat >foo/bar.sh <<EOF +#!/bin/sh +printf '%s\n' "\$@" +EOF +chmod +x foo/bar.sh + +bfs_diff . -name bar.sh -execdir {} {} \; diff --git a/tests/main.c b/tests/main.c index 4c770bd..9240e1c 100644 --- a/tests/main.c +++ b/tests/main.c @@ -222,15 +222,15 @@ int main(int argc, char *argv[]) { } tzset(); - long jobs = 0; + unsigned int jobs = 0; const char *cmd = argc > 0 ? argv[0] : "units"; int c; while (c = getopt(argc, argv, ":j:"), c != -1) { switch (c) { case 'j': - if (xstrtol(optarg, NULL, 10, &jobs) != 0 || jobs <= 0) { - fprintf(stderr, "%s: Bad job count '%s'\n", cmd, optarg); + if (xstrtoui(optarg, NULL, 10, &jobs) != 0) { + fprintf(stderr, "%s: Bad job count '%s': %s\n", cmd, optarg, errstr()); return EXIT_FAILURE; } break; @@ -243,7 +243,7 @@ int main(int argc, char *argv[]) { } } - if (jobs == 0) { + if (!jobs) { jobs = nproc(); } diff --git a/tests/posix/exec_sigmask.sh b/tests/posix/exec_sigmask.sh index d1192a4..2907458 100644 --- a/tests/posix/exec_sigmask.sh +++ b/tests/posix/exec_sigmask.sh @@ -11,6 +11,6 @@ mkfifo p1 p2 } & # Write the `sh` PID to p1, then hang reading p2 until we're killed -! invoke_bfs p1 -exec sh -c 'echo $$ >p1 && read -r _ <p2' {} + || fail +! invoke_bfs p1 -exec bash -c 'echo $$ >p1 && read -r _ <p2' bash {} + || fail -wait +_wait diff --git a/tests/posix/group_o_group.out b/tests/posix/group_o_group.out new file mode 100644 index 0000000..a7ccfe4 --- /dev/null +++ b/tests/posix/group_o_group.out @@ -0,0 +1,19 @@ +basic +basic/a +basic/b +basic/c +basic/c/d +basic/e +basic/e/f +basic/g +basic/g/h +basic/i +basic/j +basic/j/foo +basic/k +basic/k/foo +basic/k/foo/bar +basic/l +basic/l/foo +basic/l/foo/bar +basic/l/foo/bar/baz diff --git a/tests/posix/group_o_group.sh b/tests/posix/group_o_group.sh new file mode 100644 index 0000000..60aefc0 --- /dev/null +++ b/tests/posix/group_o_group.sh @@ -0,0 +1,3 @@ +# Regression test for +# https://github.com/tavianator/bfs/issues/155 +bfs_diff basic -group 0 -o -group "$(id -g)" diff --git a/tests/posix/root_order.out b/tests/posix/root_order.out new file mode 100644 index 0000000..ea94276 --- /dev/null +++ b/tests/posix/root_order.out @@ -0,0 +1,4 @@ +basic/a +basic/b +basic/c/d +basic/e/f diff --git a/tests/posix/root_order.sh b/tests/posix/root_order.sh new file mode 100644 index 0000000..86adf20 --- /dev/null +++ b/tests/posix/root_order.sh @@ -0,0 +1,6 @@ +# Root paths must be processed in order +# https://www.austingroupbugs.net/view.php?id=1859 + +# -size forces a stat(), which we don't want to be async +invoke_bfs basic/{a,b,c/d,e/f} -size -1000 >"$OUT" +diff_output diff --git a/tests/posix/user_o_user.out b/tests/posix/user_o_user.out new file mode 100644 index 0000000..a7ccfe4 --- /dev/null +++ b/tests/posix/user_o_user.out @@ -0,0 +1,19 @@ +basic +basic/a +basic/b +basic/c +basic/c/d +basic/e +basic/e/f +basic/g +basic/g/h +basic/i +basic/j +basic/j/foo +basic/k +basic/k/foo +basic/k/foo/bar +basic/l +basic/l/foo +basic/l/foo/bar +basic/l/foo/bar/baz diff --git a/tests/posix/user_o_user.sh b/tests/posix/user_o_user.sh new file mode 100644 index 0000000..7c143ae --- /dev/null +++ b/tests/posix/user_o_user.sh @@ -0,0 +1,3 @@ +# Regression test for +# https://github.com/tavianator/bfs/issues/155 +bfs_diff basic -user 0 -o -user "$(id -u)" diff --git a/tests/ptyx.c b/tests/ptyx.c index 4d89581..59292df 100644 --- a/tests/ptyx.c +++ b/tests/ptyx.c @@ -65,25 +65,21 @@ int main(int argc, char *argv[]) { exit(EXIT_FAILURE); \ } while (0) - long width = -1; - long height = -1; + unsigned short width = 0; + unsigned short height = 0; // Parse the command line int c; while (c = getopt(argc, argv, "+:w:h:"), c != -1) { switch (c) { case 'w': - if (xstrtol(optarg, NULL, 10, &width) != 0) { + if (xstrtous(optarg, NULL, 10, &width) != 0) { edie("Bad width '%s'", optarg); - } else if (width < 0 || width > (long)USHRT_MAX) { - die("Bad width '%s'", optarg); } break; case 'h': - if (xstrtol(optarg, NULL, 10, &height) != 0) { + if (xstrtous(optarg, NULL, 10, &height) != 0) { edie("Bad height '%s'", optarg); - } else if (height < 0 || height > (long)USHRT_MAX) { - die("Bad height '%s'", optarg); } break; case ':': @@ -135,36 +131,36 @@ int main(int argc, char *argv[]) { // A new pty starts at 0x0, which is not very useful. Instead, grab the // default size from the current controlling terminal, if possible. - if (width < 0 || height < 0) { + if (!width || !height) { int tty = open_cterm(O_RDONLY | O_CLOEXEC); if (tty >= 0) { struct winsize ws; if (xtcgetwinsize(tty, &ws) != 0) { edie("tcgetwinsize()"); } - if (width < 0) { + if (!width) { width = ws.ws_col; } - if (height < 0) { + if (!height) { height = ws.ws_row; } xclose(tty); } } + if (!width) { + width = 80; + } + if (!height) { + height = 24; + } - // Get the current pty size + // Update the pty size struct winsize ws; if (xtcgetwinsize(pts, &ws) != 0) { edie("tcgetwinsize()"); } - - // Apply our options - if (width >= 0) { - ws.ws_col = width; - } - if (height >= 0) { - ws.ws_row = height; - } + ws.ws_col = width; + ws.ws_row = height; if (xtcsetwinsize(pts, &ws) != 0) { edie("tcsetwinsize()"); } diff --git a/tests/run.sh b/tests/run.sh index 8d3a5d2..3ed2a9c 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -96,16 +96,13 @@ reap_test() { wait_test() { local pid line ret - while true; do + while :; do line=$((LINENO + 1)) - wait -n -ppid + _wait -n -ppid ret=$? if [ "${pid:-}" ]; then break - elif ((ret > 128)); then - # Interrupted by signal - continue else debug "${BASH_SOURCE[0]}" $line "${RED}error $ret${RST}" >&$DUPERR exit 1 diff --git a/tests/util.sh b/tests/util.sh index b846d45..1718a1a 100644 --- a/tests/util.sh +++ b/tests/util.sh @@ -190,3 +190,28 @@ pop_defers() { return $ret } + +## Parallelism + +# Get the number of processors +_nproc() { + { + nproc \ + || sysctl -n hw.ncpu \ + || getconf _NPROCESSORS_ONLN \ + || echo 1 + } 2>/dev/null +} + +# Run wait, looping if interrupted +_wait() { + local ret=130 + + # "If wait is interrupted by a signal, the return status will be greater than 128" + while ((ret > 128)); do + ret=0 + wait "$@" || ret=$? + done + + return $ret +} diff --git a/tests/xspawn.c b/tests/xspawn.c index 0244006..6864192 100644 --- a/tests/xspawn.c +++ b/tests/xspawn.c @@ -99,6 +99,22 @@ static int reset_path(char *old_path) { return ret; } +/** Spawn the test binary and check for success. */ +static void check_spawnee(const char *exe, const struct bfs_spawn *ctx, char **argv, char **envp) { + pid_t pid = bfs_spawn(exe, ctx, argv, envp); + if (!bfs_echeck(pid >= 0, "bfs_spawn('%s')", exe)) { + return; + } + + int wstatus; + bool exited = bfs_echeck(xwaitpid(pid, &wstatus, 0) == pid) + && bfs_check(WIFEXITED(wstatus)); + if (exited) { + int wexit = WEXITSTATUS(wstatus); + bfs_check(wexit == EXIT_SUCCESS, "xspawnee: exit(%d)", wexit); + } +} + /** Check that we resolve executables in $PATH correctly. */ static void check_use_path(bool use_posix) { struct bfs_spawn spawn; @@ -133,20 +149,9 @@ static void check_use_path(bool use_posix) { } char *argv[] = {"xspawnee", old_path, NULL}; - pid_t pid = bfs_spawn("xspawnee", &spawn, argv, envp); - if (!bfs_echeck(pid >= 0, "bfs_spawn()")) { - goto path; - } - - int wstatus; - bool exited = bfs_echeck(xwaitpid(pid, &wstatus, 0) == pid) - && bfs_check(WIFEXITED(wstatus)); - if (exited) { - int wexit = WEXITSTATUS(wstatus); - bfs_check(wexit == EXIT_SUCCESS, "xspawnee: exit(%d)", wexit); - } + check_spawnee("xspawnee", &spawn, argv, envp); + check_spawnee("tests/xspawnee", &spawn, argv, envp); -path: bfs_echeck(reset_path(old_path) == 0); env: for (char **var = envp; *var; ++var) { diff --git a/tests/xtouch.c b/tests/xtouch.c index 5d65a4c..f33c573 100644 --- a/tests/xtouch.c +++ b/tests/xtouch.c @@ -217,8 +217,8 @@ int main(int argc, char *argv[]) { } if (marg) { - long mode; - if (xstrtol(marg, NULL, 8, &mode) == 0 && mode >= 0 && mode < 01000) { + unsigned int mode; + if (xstrtoui(marg, NULL, 8, &mode) == 0 && mode < 01000) { args.fmode = args.dmode = mode; } else { fprintf(stderr, "%s: Invalid mode '%s'\n", cmd, marg); |