All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile
@ 2013-05-08 16:27 Mike Frysinger
  2013-05-08 16:27 ` [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user Mike Frysinger
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Mike Frysinger @ 2013-05-08 16:27 UTC (permalink / raw)
  To: linux-mtd

A bunch of utils are relying on _GNU_SOURCE already.  The new prompt code
uses getline() which is now part of POSIX, but in older versions of glibc,
it was behind _GNU_SOURCE as it was a GNU extension.

This change doesn't actually tie us to glibc.  Only code that uses GNU
extensions does that.  It just kills warning when using older versions of
glibc.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 Makefile                | 2 +-
 include/xalloc.h        | 2 +-
 mkfs.jffs2.c            | 1 -
 mkfs.ubifs/mkfs.ubifs.h | 2 --
 nanddump.c              | 1 -
 nandtest.c              | 1 -
 nandwrite.c             | 1 -
 7 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index c8d25f2..bf57a13 100644
--- a/Makefile
+++ b/Makefile
@@ -3,7 +3,7 @@
 
 VERSION = 1.5.0
 
-CPPFLAGS += -I./include -I$(BUILDDIR)/include -I./ubi-utils/include $(ZLIBCPPFLAGS) $(LZOCPPFLAGS)
+CPPFLAGS += -D_GNU_SOURCE -I./include -I$(BUILDDIR)/include -I./ubi-utils/include $(ZLIBCPPFLAGS) $(LZOCPPFLAGS)
 
 ifeq ($(WITHOUT_XATTR), 1)
   CPPFLAGS += -DWITHOUT_XATTR
diff --git a/include/xalloc.h b/include/xalloc.h
index f1cc8d4..532b80f 100644
--- a/include/xalloc.h
+++ b/include/xalloc.h
@@ -27,6 +27,7 @@
 #ifndef __MTD_UTILS_XALLOC_H__
 #define __MTD_UTILS_XALLOC_H__
 
+#include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -84,7 +85,6 @@ static char *xstrdup(const char *s)
 }
 
 #ifdef _GNU_SOURCE
-#include <stdarg.h>
 
 __attribute__((unused))
 static int xasprintf(char **strp, const char *fmt, ...)
diff --git a/mkfs.jffs2.c b/mkfs.jffs2.c
index 7ade078..c1b0f0d 100644
--- a/mkfs.jffs2.c
+++ b/mkfs.jffs2.c
@@ -49,7 +49,6 @@
 
 #define PROGRAM_NAME "mkfs.jffs2"
 
-#define _GNU_SOURCE
 #include <sys/types.h>
 #include <stdio.h>
 #include <sys/stat.h>
diff --git a/mkfs.ubifs/mkfs.ubifs.h b/mkfs.ubifs/mkfs.ubifs.h
index 01161ef..6030c48 100644
--- a/mkfs.ubifs/mkfs.ubifs.h
+++ b/mkfs.ubifs/mkfs.ubifs.h
@@ -23,8 +23,6 @@
 #ifndef __MKFS_UBIFS_H__
 #define __MKFS_UBIFS_H__
 
-#define _GNU_SOURCE
-#define _LARGEFILE64_SOURCE
 #include <unistd.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/nanddump.c b/nanddump.c
index be458c6..85ccd5d 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -15,7 +15,6 @@
 
 #define PROGRAM_NAME "nanddump"
 
-#define _GNU_SOURCE
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
diff --git a/nandtest.c b/nandtest.c
index 0187b87..3437b57 100644
--- a/nandtest.c
+++ b/nandtest.c
@@ -1,6 +1,5 @@
 #define PROGRAM_NAME "nandtest"
 
-#define _GNU_SOURCE
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
diff --git a/nandwrite.c b/nandwrite.c
index de8e7d2..a6b6581 100644
--- a/nandwrite.c
+++ b/nandwrite.c
@@ -21,7 +21,6 @@
 
 #define PROGRAM_NAME "nandwrite"
 
-#define _GNU_SOURCE
 #include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user
  2013-05-08 16:27 [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Mike Frysinger
@ 2013-05-08 16:27 ` Mike Frysinger
  2013-07-04  2:01   ` Brian Norris
  2013-05-08 16:27 ` [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices Mike Frysinger
  2013-07-01  5:57 ` [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Artem Bityutskiy
  2 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2013-05-08 16:27 UTC (permalink / raw)
  To: linux-mtd

We've got a few tools that prompt the user for "yes/no" questions.
Add a common helper to simplify the various implementations.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 flash_otp_lock.c      |  6 +++---
 ftl_format.c          | 11 +++--------
 include/common.h      | 39 ++++++++++++++++++++++++++++++++++++++-
 ubi-utils/ubiformat.c | 39 +++++++--------------------------------
 4 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/flash_otp_lock.c b/flash_otp_lock.c
index cc8759e..3c39a2d 100644
--- a/flash_otp_lock.c
+++ b/flash_otp_lock.c
@@ -13,11 +13,12 @@
 #include <sys/ioctl.h>
 
 #include <mtd/mtd-user.h>
+#include "common.h"
 
 int main(int argc,char *argv[])
 {
 	int fd, val, ret, offset, size;
-	char *p, buf[8];
+	char *p;
 
 	if (argc != 5 || strcmp(argv[1], "-u")) {
 		fprintf(stderr, "Usage: %s -u <device> <offset> <size>\n", PROGRAM_NAME);
@@ -53,8 +54,7 @@ int main(int argc,char *argv[])
 
 	printf("About to lock OTP user data on %s from 0x%x to 0x%x\n",
 			argv[2], offset, offset + size);
-	printf("Are you sure (yes|no)? ");
-	if (fgets(buf, sizeof(buf), stdin) && strcmp(buf, "yes\n") == 0) {
+	if (prompt("Are you sure?", false)) {
 		struct otp_info info;
 		info.start = offset;
 		info.length = size;
diff --git a/ftl_format.c b/ftl_format.c
index 0f69b8f..b58677f 100644
--- a/ftl_format.c
+++ b/ftl_format.c
@@ -51,6 +51,7 @@
 #include <mtd/mtd-user.h>
 #include <mtd/ftl-user.h>
 #include <mtd_swab.h>
+#include "common.h"
 
 /*====================================================================*/
 
@@ -164,15 +165,9 @@ static int format_partition(int fd, int quiet, int interrogate,
 		fflush(stdout);
 	}
 
-	if (interrogate) {
-		char str[3];
-		printf("This will destroy all data on the target device.  "
-				"Confirm (y/n): ");
-		if (fgets(str, 3, stdin) == NULL)
+	if (interrogate)
+		if (!prompt("This will destroy all data on the target device. Confirm?", false))
 			return -1;
-		if ((strcmp(str, "y\n") != 0) && (strcmp(str, "Y\n") != 0))
-			return -1;
-	}
 
 	/* Create basic block allocation table for control blocks */
 	nbam = ((mtd.erasesize >> hdr.BlockSize) * sizeof(u_int)
diff --git a/include/common.h b/include/common.h
index d0c4146..4ffccea 100644
--- a/include/common.h
+++ b/include/common.h
@@ -19,6 +19,7 @@
 #ifndef __MTD_UTILS_COMMON_H__
 #define __MTD_UTILS_COMMON_H__
 
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <ctype.h>
@@ -101,9 +102,45 @@ extern "C" {
 	fprintf(stderr, "%s: warning!: " fmt "\n", PROGRAM_NAME, ##__VA_ARGS__); \
 } while(0)
 
+/**
+ * prompt the user for confirmation
+ */
+static inline bool prompt(const char *msg, bool def)
+{
+	char *line = NULL;
+	size_t len;
+	bool ret = def;
+
+	do {
+		normsg_cont("%s (%c/%c) ", msg, def ? 'Y' : 'y', def ? 'n' : 'N');
+		fflush(stdout);
+
+		while (getline(&line, &len, stdin) == -1) {
+			printf("failed to read prompt; assuming '%s'\n",
+				def ? "yes" : "no");
+			break;
+		}
+
+		if (strcmp("\n", line) != 0) {
+			switch (rpmatch(line)) {
+			case 0: ret = false; break;
+			case 1: ret = true; break;
+			case -1:
+				puts("unknown response; please try again");
+				continue;
+			}
+		}
+		break;
+	} while (1);
+
+	free(line);
+
+	return ret;
+}
+
 static inline int is_power_of_2(unsigned long long n)
 {
-	        return (n != 0 && ((n & (n - 1)) == 0));
+	return (n != 0 && ((n & (n - 1)) == 0));
 }
 
 /**
diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
index 899f9fc..97a4eab 100644
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -243,35 +243,12 @@ static int parse_opt(int argc, char * const argv[])
 
 static int want_exit(void)
 {
-	char buf[4];
-
-	while (1) {
-		normsg_cont("continue? (yes/no)  ");
-		if (scanf("%3s", buf) == EOF) {
-			sys_errmsg("scanf returned unexpected EOF, assume \"yes\"");
-			return 1;
-		}
-		if (!strncmp(buf, "yes", 3) || !strncmp(buf, "y", 1))
-			return 0;
-		if (!strncmp(buf, "no", 2) || !strncmp(buf, "n", 1))
-			return 1;
-	}
+	return prompt("continue?", false) == true ? 0 : 1;
 }
 
-static int answer_is_yes(void)
+static int answer_is_yes(const char *msg)
 {
-	char buf[4];
-
-	while (1) {
-		if (scanf("%3s", buf) == EOF) {
-			sys_errmsg("scanf returned unexpected EOF, assume \"no\"");
-			return 0;
-		}
-		if (!strncmp(buf, "yes", 3) || !strncmp(buf, "y", 1))
-			return 1;
-		if (!strncmp(buf, "no", 2) || !strncmp(buf, "n", 1))
-			return 0;
-	}
+	return prompt(msg ? : "continue?", false);
 }
 
 static void print_bad_eraseblocks(const struct mtd_dev_info *mtd,
@@ -412,11 +389,9 @@ static int mark_bad(const struct mtd_dev_info *mtd, struct ubi_scan_info *si, in
 {
 	int err;
 
-	if (!args.yes) {
-		normsg_cont("mark it as bad? Continue (yes/no) ");
-		if (!answer_is_yes())
+	if (!args.yes)
+		if (!answer_is_yes("mark it as bad?"))
 			return -1;
-	}
 
 	if (!args.quiet)
 		normsg_cont("marking block %d bad", eb);
@@ -922,10 +897,10 @@ int main(int argc, char * const argv[])
 				"which is different to requested offsets %d and %d",
 				si->vid_hdr_offs, si->data_offs, ui.vid_hdr_offs,
 				ui.data_offs);
-			normsg_cont("use new offsets %d and %d? (yes/no)  ",
+			normsg_cont("use new offsets %d and %d? ",
 				    ui.vid_hdr_offs, ui.data_offs);
 		}
-		if (args.yes || answer_is_yes()) {
+		if (args.yes || answer_is_yes(NULL)) {
 			if (args.yes && !args.quiet)
 				printf("yes\n");
 		} else
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices
  2013-05-08 16:27 [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Mike Frysinger
  2013-05-08 16:27 ` [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user Mike Frysinger
@ 2013-05-08 16:27 ` Mike Frysinger
  2013-05-10  7:07   ` Ricard Wanderlof
  2013-07-01  5:57 ` [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Artem Bityutskiy
  2 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2013-05-08 16:27 UTC (permalink / raw)
  To: linux-mtd

Sometimes I want to re-initialize an existing ubifs, but the tool
currently bails out if the volume is already formatted.  Prompt the
user instead so they can decide.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 mkfs.ubifs/mkfs.ubifs.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/mkfs.ubifs/mkfs.ubifs.c b/mkfs.ubifs/mkfs.ubifs.c
index 2bb819e..427e37d 100644
--- a/mkfs.ubifs/mkfs.ubifs.c
+++ b/mkfs.ubifs/mkfs.ubifs.c
@@ -103,6 +103,7 @@ static libubi_t ubi;
 /* Debug levels are: 0 (none), 1 (statistics), 2 (files) ,3 (more details) */
 int debug_level;
 int verbose;
+int yes;
 
 static char *root;
 static int root_len;
@@ -133,7 +134,7 @@ static struct inum_mapping **hash_table;
 /* Inode creation sequence number */
 static unsigned long long creat_sqnum;
 
-static const char *optstring = "d:r:m:o:D:h?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQq";
+static const char *optstring = "d:r:m:o:D:yh?vVe:c:g:f:Fp:k:x:X:j:R:l:j:UQq";
 
 static const struct option longopts[] = {
 	{"root",               1, NULL, 'r'},
@@ -142,6 +143,7 @@ static const struct option longopts[] = {
 	{"max-leb-cnt",        1, NULL, 'c'},
 	{"output",             1, NULL, 'o'},
 	{"devtable",           1, NULL, 'D'},
+	{"yes",                0, NULL, 'y'},
 	{"help",               0, NULL, 'h'},
 	{"verbose",            0, NULL, 'v'},
 	{"version",            0, NULL, 'V'},
@@ -191,6 +193,7 @@ static const char *helptext =
 "-U, --squash-uids        squash owners making all files owned by root\n"
 "-l, --log-lebs=COUNT     count of erase blocks for the log (used only for\n"
 "                         debugging)\n"
+"-y, --yes                assume the answer is \"yes\" for all questions\n"
 "-v, --verbose            verbose operation\n"
 "-V, --version            display version information\n"
 "-g, --debug=LEVEL        display debug information (0 - none, 1 - statistics,\n"
@@ -539,6 +542,9 @@ static int get_options(int argc, char**argv)
 				return sys_err_msg("bad device table file '%s'",
 						   tbl_file);
 			break;
+		case 'y':
+			yes = 1;
+			break;
 		case 'h':
 		case '?':
 			printf("%s", helptext);
@@ -2098,8 +2104,10 @@ static int open_target(void)
 		if (ubi_set_property(out_fd, UBI_VOL_PROP_DIRECT_WRITE, 1))
 			return sys_err_msg("ubi_set_property failed");
 
-		if (check_volume_empty())
-			return err_msg("UBI volume is not empty");
+		if (!yes && check_volume_empty()) {
+			if (!prompt("UBI volume is not empty.  Format anyways?", false))
+				return err_msg("UBI volume is not empty");
+		}
 	} else {
 		out_fd = open(output, O_CREAT | O_RDWR | O_TRUNC,
 			      S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices
  2013-05-08 16:27 ` [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices Mike Frysinger
@ 2013-05-10  7:07   ` Ricard Wanderlof
  2013-05-10 15:26     ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Ricard Wanderlof @ 2013-05-10  7:07 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-mtd


On Wed, 8 May 2013, Mike Frysinger wrote:

> Sometimes I want to re-initialize an existing ubifs, but the tool
> currently bails out if the volume is already formatted.  Prompt the
> user instead so they can decide.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> ...
> 	{"root",               1, NULL, 'r'},
> @@ -142,6 +143,7 @@ static const struct option longopts[] = {
> 	{"max-leb-cnt",        1, NULL, 'c'},
> 	{"output",             1, NULL, 'o'},
> 	{"devtable",           1, NULL, 'D'},
> +	{"yes",                0, NULL, 'y'},
> 	{"help",               0, NULL, 'h'},
> 	{"verbose",            0, NULL, 'v'},
> 	{"version",            0, NULL, 'V'},
> @@ -191,6 +193,7 @@ static const char *helptext =
> "-U, --squash-uids        squash owners making all files owned by root\n"
> "-l, --log-lebs=COUNT     count of erase blocks for the log (used only for\n"
> "                         debugging)\n"
> +"-y, --yes                assume the answer is \"yes\" for all questions\n"
> ...

Wouldn't it be better to have a specific option for this specific case, 
rather than a general yes-to-everything option? The latter makes sense 
with programs such as fsck where the only prompt basically is 'I found a 
fault, shall I fix it?', but in this is case it can be difficult to 
predict the outcome should the option start to cover more potential 
questions in the future.

-f and -F are already taken, but I was thinking something along the lines of:

"-T, --reformat		format an existing ubifs even if already formatted"

/Ricard
-- 
Ricard Wolf Wanderlöf                           ricardw(at)axis.com
Axis Communications AB, Lund, Sweden            www.axis.com
Phone +46 46 272 2016                           Fax +46 46 13 61 30

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices
  2013-05-10  7:07   ` Ricard Wanderlof
@ 2013-05-10 15:26     ` Mike Frysinger
  2013-05-13  8:14       ` Artem Bityutskiy
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2013-05-10 15:26 UTC (permalink / raw)
  To: Ricard Wanderlof; +Cc: linux-mtd

[-- Attachment #1: Type: Text/Plain, Size: 1603 bytes --]

On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> On Wed, 8 May 2013, Mike Frysinger wrote:
> > Sometimes I want to re-initialize an existing ubifs, but the tool
> > currently bails out if the volume is already formatted.  Prompt the
> > user instead so they can decide.
> > 
> > 	{"max-leb-cnt",        1, NULL, 'c'},
> > 	{"output",             1, NULL, 'o'},
> > 	{"devtable",           1, NULL, 'D'},
> > +	{"yes",                0, NULL, 'y'},
> > 	{"help",               0, NULL, 'h'},
> > 	{"verbose",            0, NULL, 'v'},
> > 	{"version",            0, NULL, 'V'},
> > 
> > @@ -191,6 +193,7 @@ static const char *helptext =
> > "-U, --squash-uids        squash owners making all files owned by root\n"
> > "-l, --log-lebs=COUNT     count of erase blocks for the log (used only
> > for\n" "                         debugging)\n"
> > +"-y, --yes                assume the answer is \"yes\" for all
> > questions\n" ...
> 
> Wouldn't it be better to have a specific option for this specific case,
> rather than a general yes-to-everything option?

this is the standard that the various mtd tools (including a bunch of UBI 
ones) follow

> The latter makes sense
> with programs such as fsck where the only prompt basically is 'I found a
> fault, shall I fix it?', but in this is case it can be difficult to
> predict the outcome should the option start to cover more potential
> questions in the future.

if you want to compare to standard tools, then the check would be dropped 
entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices
  2013-05-10 15:26     ` Mike Frysinger
@ 2013-05-13  8:14       ` Artem Bityutskiy
  2013-05-13 16:10         ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Artem Bityutskiy @ 2013-05-13  8:14 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-mtd, Ricard Wanderlof

On Fri, 2013-05-10 at 11:26 -0400, Mike Frysinger wrote:
> On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> > On Wed, 8 May 2013, Mike Frysinger wrote:
> > > Sometimes I want to re-initialize an existing ubifs, but the tool
> > > currently bails out if the volume is already formatted.  Prompt the
> > > user instead so they can decide.
> > > 
> > >     {"max-leb-cnt",        1, NULL, 'c'},
> > >     {"output",             1, NULL, 'o'},
> > >     {"devtable",           1, NULL, 'D'},
> > > +   {"yes",                0, NULL, 'y'},
> > >     {"help",               0, NULL, 'h'},
> > >     {"verbose",            0, NULL, 'v'},
> > >     {"version",            0, NULL, 'V'},
> > > 
> > > @@ -191,6 +193,7 @@ static const char *helptext =
> > > "-U, --squash-uids        squash owners making all files owned by root\n"
> > > "-l, --log-lebs=COUNT     count of erase blocks for the log (used only
> > > for\n" "                         debugging)\n"
> > > +"-y, --yes                assume the answer is \"yes\" for all
> > > questions\n" ...
> > 
> > Wouldn't it be better to have a specific option for this specific case,
> > rather than a general yes-to-everything option?
> 
> this is the standard that the various mtd tools (including a bunch of UBI 
> ones) follow
> 
> > The latter makes sense
> > with programs such as fsck where the only prompt basically is 'I found a
> > fault, shall I fix it?', but in this is case it can be difficult to
> > predict the outcome should the option start to cover more potential
> > questions in the future.
> 
> if you want to compare to standard tools, then the check would be dropped 
> entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.

I admit I was not careful enough with the options. Feel free to change
the tools in the more direction of being more consistent with mainstream
filesystems' tools. 

-- 
Best Regards,
Artem Bityutskiy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices
  2013-05-13  8:14       ` Artem Bityutskiy
@ 2013-05-13 16:10         ` Mike Frysinger
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Frysinger @ 2013-05-13 16:10 UTC (permalink / raw)
  To: dedekind1; +Cc: linux-mtd, Ricard Wanderlof

[-- Attachment #1: Type: Text/Plain, Size: 2267 bytes --]

On Monday 13 May 2013 04:14:08 Artem Bityutskiy wrote:
> On Fri, 2013-05-10 at 11:26 -0400, Mike Frysinger wrote:
> > On Friday 10 May 2013 03:07:53 Ricard Wanderlof wrote:
> > > On Wed, 8 May 2013, Mike Frysinger wrote:
> > > > Sometimes I want to re-initialize an existing ubifs, but the tool
> > > > currently bails out if the volume is already formatted.  Prompt the
> > > > user instead so they can decide.
> > > > 
> > > >     {"max-leb-cnt",        1, NULL, 'c'},
> > > >     {"output",             1, NULL, 'o'},
> > > >     {"devtable",           1, NULL, 'D'},
> > > > 
> > > > +   {"yes",                0, NULL, 'y'},
> > > > 
> > > >     {"help",               0, NULL, 'h'},
> > > >     {"verbose",            0, NULL, 'v'},
> > > >     {"version",            0, NULL, 'V'},
> > > > 
> > > > @@ -191,6 +193,7 @@ static const char *helptext =
> > > > "-U, --squash-uids        squash owners making all files owned by
> > > > root\n" "-l, --log-lebs=COUNT     count of erase blocks for the log
> > > > (used only for\n" "                         debugging)\n"
> > > > +"-y, --yes                assume the answer is \"yes\" for all
> > > > questions\n" ...
> > > 
> > > Wouldn't it be better to have a specific option for this specific case,
> > > rather than a general yes-to-everything option?
> > 
> > this is the standard that the various mtd tools (including a bunch of UBI
> > ones) follow
> > 
> > > The latter makes sense
> > > with programs such as fsck where the only prompt basically is 'I found
> > > a fault, shall I fix it?', but in this is case it can be difficult to
> > > predict the outcome should the option start to cover more potential
> > > questions in the future.
> > 
> > if you want to compare to standard tools, then the check would be dropped
> > entirely.  when i run `mke2fs /dev/sda1`, it doesn't prompt me.
> 
> I admit I was not careful enough with the options. Feel free to change
> the tools in the more direction of being more consistent with mainstream
> filesystems' tools.

i don't mind having it prompting first as long as there is a flag to override 
it.  i'd say let's take a survey of the community, but i think we both know 
that's doomed to go nowhere :).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile
  2013-05-08 16:27 [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Mike Frysinger
  2013-05-08 16:27 ` [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user Mike Frysinger
  2013-05-08 16:27 ` [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices Mike Frysinger
@ 2013-07-01  5:57 ` Artem Bityutskiy
  2 siblings, 0 replies; 12+ messages in thread
From: Artem Bityutskiy @ 2013-07-01  5:57 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-mtd

On Wed, 2013-05-08 at 12:27 -0400, Mike Frysinger wrote:
> A bunch of utils are relying on _GNU_SOURCE already.  The new prompt code
> uses getline() which is now part of POSIX, but in older versions of glibc,
> it was behind _GNU_SOURCE as it was a GNU extension.
> 
> This change doesn't actually tie us to glibc.  Only code that uses GNU
> extensions does that.  It just kills warning when using older versions of
> glibc.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Pushed all 3 to mtd-utils.

-- 
Best Regards,
Artem Bityutskiy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user
  2013-05-08 16:27 ` [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user Mike Frysinger
@ 2013-07-04  2:01   ` Brian Norris
  2013-08-24 23:19     ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Brian Norris @ 2013-07-04  2:01 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-mtd, Artem Bityutskiy

Hi Mike, Artem,

I notice Artem just pushed this.

On Wed, May 8, 2013 at 9:27 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> We've got a few tools that prompt the user for "yes/no" questions.
> Add a common helper to simplify the various implementations.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
...
> diff --git a/include/common.h b/include/common.h
> index d0c4146..4ffccea 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -101,9 +102,45 @@ extern "C" {
>         fprintf(stderr, "%s: warning!: " fmt "\n", PROGRAM_NAME, ##__VA_ARGS__); \
>  } while(0)
>
> +/**
> + * prompt the user for confirmation
> + */
> +static inline bool prompt(const char *msg, bool def)
> +{
> +       char *line = NULL;
> +       size_t len;
> +       bool ret = def;
> +
> +       do {
> +               normsg_cont("%s (%c/%c) ", msg, def ? 'Y' : 'y', def ? 'n' : 'N');
> +               fflush(stdout);
> +
> +               while (getline(&line, &len, stdin) == -1) {
> +                       printf("failed to read prompt; assuming '%s'\n",
> +                               def ? "yes" : "no");
> +                       break;
> +               }
> +
> +               if (strcmp("\n", line) != 0) {
> +                       switch (rpmatch(line)) {

rpmatch() is not POSIX-compliant and is not available on my uclibc, so
this breaks my compile.

> +                       case 0: ret = false; break;
> +                       case 1: ret = true; break;
> +                       case -1:
> +                               puts("unknown response; please try again");
> +                               continue;
> +                       }
...

Brian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user
  2013-07-04  2:01   ` Brian Norris
@ 2013-08-24 23:19     ` Mike Frysinger
  2013-09-17  0:35       ` Brian Norris
  0 siblings, 1 reply; 12+ messages in thread
From: Mike Frysinger @ 2013-08-24 23:19 UTC (permalink / raw)
  To: Brian Norris; +Cc: linux-mtd, Artem Bityutskiy

[-- Attachment #1: Type: Text/Plain, Size: 450 bytes --]

On Wednesday 03 July 2013 22:01:52 Brian Norris wrote:
> On Wed, May 8, 2013 at 9:27 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > --- a/include/common.h
> > +++ b/include/common.h
> > 
> > +               if (strcmp("\n", line) != 0) {
> > +                       switch (rpmatch(line)) {
> 
> rpmatch() is not POSIX-compliant and is not available on my uclibc, so
> this breaks my compile.

i added this to uClibc already :)
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user
  2013-08-24 23:19     ` Mike Frysinger
@ 2013-09-17  0:35       ` Brian Norris
  2013-09-30  0:54         ` Mike Frysinger
  0 siblings, 1 reply; 12+ messages in thread
From: Brian Norris @ 2013-09-17  0:35 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: linux-mtd, Artem Bityutskiy

On Sat, Aug 24, 2013 at 07:19:42PM -0400, Mike Frysinger wrote:
> On Wednesday 03 July 2013 22:01:52 Brian Norris wrote:
> > On Wed, May 8, 2013 at 9:27 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > > --- a/include/common.h
> > > +++ b/include/common.h
> > > 
> > > +               if (strcmp("\n", line) != 0) {
> > > +                       switch (rpmatch(line)) {
> > 
> > rpmatch() is not POSIX-compliant and is not available on my uclibc, so
> > this breaks my compile.
> 
> i added this to uClibc already :)

So I have to upgrade uClibc if I'm going to upgrade mtd-utils. Nice! And
to top it off, Huang is adding some NAND SLC/MLC changes that will
require an mtd-utils update soon.

I should be moving away from uClibc soon enough, so this isn't too
important for me to whine about.

Brian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user
  2013-09-17  0:35       ` Brian Norris
@ 2013-09-30  0:54         ` Mike Frysinger
  0 siblings, 0 replies; 12+ messages in thread
From: Mike Frysinger @ 2013-09-30  0:54 UTC (permalink / raw)
  To: Brian Norris; +Cc: linux-mtd, Artem Bityutskiy

[-- Attachment #1: Type: Text/Plain, Size: 844 bytes --]

On Monday 16 September 2013 20:35:49 Brian Norris wrote:
> On Sat, Aug 24, 2013 at 07:19:42PM -0400, Mike Frysinger wrote:
> > On Wednesday 03 July 2013 22:01:52 Brian Norris wrote:
> > > On Wed, May 8, 2013 at 9:27 AM, Mike Frysinger wrote:
> > > > --- a/include/common.h
> > > > +++ b/include/common.h
> > > > 
> > > > +               if (strcmp("\n", line) != 0) {
> > > > +                       switch (rpmatch(line)) {
> > > 
> > > rpmatch() is not POSIX-compliant and is not available on my uclibc, so
> > > this breaks my compile.
> > 
> > i added this to uClibc already :)
> 
> So I have to upgrade uClibc if I'm going to upgrade mtd-utils. Nice!

it's a one line change to a header file that you could easily do locally.  no 
need to be facetious.

#define rpmatch(line) (line[0] == 'Y' || line[0] == 'y')
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-09-30  0:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-08 16:27 [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Mike Frysinger
2013-05-08 16:27 ` [PATCH [mtd-utils] 2/3] mtd-utils: new prompt() helper for talking to the user Mike Frysinger
2013-07-04  2:01   ` Brian Norris
2013-08-24 23:19     ` Mike Frysinger
2013-09-17  0:35       ` Brian Norris
2013-09-30  0:54         ` Mike Frysinger
2013-05-08 16:27 ` [PATCH [mtd-utils] 3/3] mkfs.ubifs: allow reformatting of devices Mike Frysinger
2013-05-10  7:07   ` Ricard Wanderlof
2013-05-10 15:26     ` Mike Frysinger
2013-05-13  8:14       ` Artem Bityutskiy
2013-05-13 16:10         ` Mike Frysinger
2013-07-01  5:57 ` [PATCH [mtd-utils] 1/3] move _GNU_SOURCE to the main makefile Artem Bityutskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.