linux-mtd.lists.infradead.org archive mirror
 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@lists.infradead.org


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@lists.infradead.org

[-- 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@lists.infradead.org, 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@lists.infradead.org, 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).