Hi! Your changes to the patch look fine, apart from one detail. The reason why the applet checks for block deviceness before open(2) is that the behavior of open(2) with the O_EXCL flag, but without the O_CREAT flag, is undefined unless the file is a block device. It is your call whether this is a sufficient reason for increasing the code size a bit.
Best regards, Ari Sundholm [email protected] On Sat, 2016-01-02 at 00:21 +0000, Denys Vlasenko wrote: > Applied, thanks! > > Please try and yell at me if something's not right. > > On Tue, Dec 1, 2015 at 2:55 PM, Ari Sundholm <[email protected]> wrote: > > From: Ari Sundholm <[email protected]> > > > > This applet discards blocks on a given device. > > Tested with multiple 32-bit and 64-bit kernel versions and two different > > SSDs. > > > > function old new delta > > blkdiscard_main - 300 +300 > > .rodata 154690 154795 +105 > > packed_usage 30275 30310 +35 > > applet_names 2499 2510 +11 > > applet_main 2904 2912 +8 > > applet_nameofs 726 728 +2 > > ------------------------------------------------------------------------------ > > (add/remove: 2/0 grow/shrink: 5/0 up/down: 461/0) Total: 461 > > bytes > > > > v2: Use a common error message format string when !S_ISBLK() to save space. > > v3: Make the applet configuration option default to n and add a comment > > regarding two not strictly necessary variables which help readability. > > > > Signed-off-by: Ari Sundholm <[email protected]> > > --- > > util-linux/blkdiscard.c | 86 > > +++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 86 insertions(+) > > create mode 100644 util-linux/blkdiscard.c > > > > diff --git a/util-linux/blkdiscard.c b/util-linux/blkdiscard.c > > new file mode 100644 > > index 0000000..0d28da2 > > --- /dev/null > > +++ b/util-linux/blkdiscard.c > > @@ -0,0 +1,86 @@ > > +/* > > + * Mini blkdiscard implementation for busybox > > + * > > + * Copyright (C) 2015 by Ari Sundholm <[email protected]> and Tuxera Inc. > > + * > > + * Licensed under GPLv2 or later, see file LICENSE in this source tree. > > + */ > > + > > +//config:config BLKDISCARD > > +//config: bool "blkdiscard" > > +//config: default n > > +//config: help > > +//config: blkdiscard discards sectors on a given device. > > + > > +//kbuild:lib-$(CONFIG_BLKDISCARD) += blkdiscard.o > > +//applet:IF_BLKDISCARD(APPLET_NOFORK(blkdiscard, blkdiscard, > > BB_DIR_USR_BIN, BB_SUID_DROP, blkdiscard)) > > + > > +//usage:#define blkdiscard_trivial_usage > > +//usage: "[-o offset] [-l length] [-s] device" > > +//usage:#define blkdiscard_full_usage "\n\n" > > +//usage: "Discard sectors on the given device\n" > > +//usage: "\n -o offset Byte offset into device (default: > > 0)" > > +//usage: "\n -l length Number of bytes to discard > > (default: size of device)" > > +//usage: "\n -s Perform a secure discard" > > +//usage: > > +//usage:#define blkdiscard_example_usage > > +//usage: "$ blkdiscard -o 0 -l 1G /dev/sdb" > > + > > +#include "libbb.h" > > +#include <linux/fs.h> > > + > > +/* This is a NOFORK applet. Be very careful! */ > > + > > +int blkdiscard_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; > > +int blkdiscard_main(int argc UNUSED_PARAM, char **argv) > > +{ > > + unsigned opts; > > + char *offset_str; > > + char *length_str; > > + uint64_t offset = 0; /* Leaving these two variables out does not */ > > + uint64_t length; /* shrink code size and hampers readability. */ > > + uint64_t range[2]; > > + struct stat st; > > + int fd; > > + > > + enum { > > + OPT_OFFSET = (1 << 0), > > + OPT_LENGTH = (1 << 1), > > + OPT_SECURE = (1 << 2), > > + }; > > + > > + opt_complementary = "=1"; > > + opts = getopt32(argv, "o:l:s", &offset_str, &length_str); > > + > > + xstat(argv[optind], &st); > > + if (!S_ISBLK(st.st_mode)) > > + bb_error_msg_and_die("%s: not a block device", > > argv[optind]); > > + > > + fd = xopen(argv[optind], O_RDWR|O_EXCL); > > + > > + if (opts & OPT_OFFSET) > > + offset = xatoull_sfx(offset_str, kMG_suffixes); > > + > > + if (opts & OPT_LENGTH) > > + length = xatoull_sfx(length_str, kMG_suffixes); > > + else { > > + ioctl_or_perror_and_die(fd, BLKGETSIZE64, &length, > > + "Failed to get device size"); > > + length -= offset; > > + } > > + > > + range[0] = offset; > > + range[1] = length; > > + ioctl_or_perror_and_die(fd, > > + (opts & OPT_SECURE) > > + ? BLKSECDISCARD > > + : BLKDISCARD, &range, > > + "%s failed", > > + (opts & OPT_SECURE) > > + ? "BLKSECDISCARD" > > + : "BLKDISCARD"); > > + > > + xclose(fd); > > + > > + return EXIT_SUCCESS; > > +} > > -- > > 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
