On Tue, 2015-07-21 at 19:52 +0200, Denys Vlasenko wrote: > I reworked it so that these additions are optional, > and applied the result.
Looks great! I was a bit worried about the bloat-o-meter values, but you were able to shrink the changes down considerably. Just one minor thing that I noticed. When running "busybox sync -f" or "busybox sync -d", that is, with an option but no files, sync() is called. This may be a bit unexpected and can easily result from something like "busybox sync -f `ls`" in a shell script in an empty directory (not that this is good style). Is this behavior what you intended? > Thanks! Thank you. > On Mon, Jul 20, 2015 at 6:55 PM, Ari Sundholm <[email protected]> wrote: > > From: Ari Sundholm <[email protected]> > > > > This brings busybox in line with modern coreutils sync. > > > > function old new delta > > sync_main 19 214 +195 > > .rodata 155261 155373 +112 > > packed_usage 30228 30270 +42 > > ------------------------------------------------------------------------------ > > (add/remove: 0/0 grow/shrink: 3/0 up/down: 349/0) Total: 349 > > bytes > > > > Signed-off-by: Ari Sundholm <[email protected]> > > --- > > coreutils/Config.src | 6 ---- > > coreutils/sync.c | 87 > > ++++++++++++++++++++++++++++++++++++++++++++++++---- > > 2 files changed, 81 insertions(+), 12 deletions(-) > > > > diff --git a/coreutils/Config.src b/coreutils/Config.src > > index 1ec3a0a..02155d2 100644 > > --- a/coreutils/Config.src > > +++ b/coreutils/Config.src > > @@ -571,12 +571,6 @@ config SUM > > help > > checksum and count the blocks in a file > > > > -config SYNC > > - bool "sync" > > - default y > > - help > > - sync is used to flush filesystem buffers. > > - > > config TAC > > bool "tac" > > default y > > diff --git a/coreutils/sync.c b/coreutils/sync.c > > index 7d98a1e..c5fdc5e 100644 > > --- a/coreutils/sync.c > > +++ b/coreutils/sync.c > > @@ -3,16 +3,40 @@ > > * Mini sync implementation for busybox > > * > > * Copyright (C) 1995, 1996 by Bruce Perens <[email protected]>. > > + * Copyright (C) 2015 by Ari Sundholm <[email protected]> > > * > > * Licensed under GPLv2 or later, see file LICENSE in this source tree. > > */ > > > > /* BB_AUDIT SUSv3 N/A -- Matches GNU behavior. */ > > +//config:config SYNC > > +//config: bool "sync" > > +//config: default y > > +//config: help > > +//config: sync is used to flush filesystem buffers. > > +//config:config FEATURE_SYNC_SYNCFS > > +//config: bool "Enable the use of syncfs(2)" > > +//config: default y > > +//config: depends on SYNC > > +//config: help > > +//config: Enables the sync applet to use syncfs(2) to offer the > > additional > > +//config: -f flag, which allows for synchronizing the filesystems > > underlying > > +//config: a set of files. > > > > //usage:#define sync_trivial_usage > > -//usage: "" > > +//usage: "[-d" > > +//usage: IF_FEATURE_SYNC_SYNCFS( > > +//usage: "|-f" > > +//usage: ) > > +//usage: "] [FILE ...]" > > //usage:#define sync_full_usage "\n\n" > > -//usage: "Write all buffered blocks to disk" > > +//usage: "Write all buffered blocks in FILEs or all filesystems to > > disk" > > +//usage: "\n -d Avoid syncing metadata" > > +//usage: IF_FEATURE_SYNC_SYNCFS( > > +//usage: "\n -f Sync underlying filesystem" > > +//usage: ) > > +//usage: > > + > > > > #include "libbb.h" > > > > @@ -21,10 +45,61 @@ > > int sync_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; > > int sync_main(int argc UNUSED_PARAM, char **argv > > IF_NOT_DESKTOP(UNUSED_PARAM)) > > { > > - /* coreutils-6.9 compat */ > > - bb_warn_ignoring_args(argv[1]); > > + unsigned opts; > > + int ret = EXIT_SUCCESS; > > + > > + enum { > > + OPT_DATASYNC = (1 << 0), > > +#if ENABLE_FEATURE_SYNC_SYNCFS > > + OPT_SYNCFS = (1 << 1), > > +#endif > > + }; > > + > > + opt_complementary = > > +#if ENABLE_FEATURE_SYNC_SYNCFS > > + "d--f:f--d:d--d:f--f"; > > +#else > > + "d--d"; > > +#endif > > + opts = getopt32(argv, > > + "d" > > +#if ENABLE_FEATURE_SYNC_SYNCFS > > + "f" > > +#endif > > + ); > > + > > + argv += optind; > > + > > + /* Handle the no-argument case. */ > > + if (!*argv && !(opts & OPT_DATASYNC) > > +#if ENABLE_FEATURE_SYNC_SYNCFS > > + && !(opts & OPT_SYNCFS) > > +#endif > > + ) > > + sync(); > > + > > + while (*argv) { > > + int fd = open(*argv, O_RDONLY); > > > > - sync(); > > + if (fd < 0) { > > + bb_perror_msg("%s: open", *argv); > > + ret = EXIT_FAILURE; > > + } else { > > + if (opts & OPT_DATASYNC && fdatasync(fd) < 0) { > > + bb_perror_msg("%s: fdatasync", *argv); > > + ret = EXIT_FAILURE; > > +#if ENABLE_FEATURE_SYNC_SYNCFS > > + } else if (opts & OPT_SYNCFS && syncfs(fd) < 0) { > > + bb_perror_msg("%s: syncfs", *argv); > > + ret = EXIT_FAILURE; > > +#endif > > + } else if (fsync(fd) < 0) { > > + bb_perror_msg("%s: fsync", *argv); > > + ret = EXIT_FAILURE; > > + } > > + } > > + ++argv; > > + } > > > > - return EXIT_SUCCESS; > > + return ret; > > } > > -- > > 1.9.1 > > > > > > > > _______________________________________________ > > busybox mailing list > > [email protected] > > http://lists.busybox.net/mailman/listinfo/busybox _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
