diff mbox

[v2,1/2] btrfs-progs: Introduce warning and error for common use

Message ID e4768fb11c5e7584b2211559c26d974517e70171.1442396165.git.zhaolei@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhaolei Sept. 16, 2015, 9:40 a.m. UTC
Current code use fprintf(stderr, "...") to output warnning and
error information.

The error message have different style, as:
 # grep fprintf *.c
 fprintf(stderr, "Open ctree failed\n");
 fprintf(stderr, "%s: open ctree failed\n", __func__);
 fprintf(stderr, "ERROR: cannot open ctree\n");
 ...

And sometimes, we forgot add tailed '\n', or use printf instead,
as in current code:
 printf("warning, device %llu is missing\n",

This patch introduce warning() and error() as common function,
to make:
1: Each warning and error information have same format
2: Easy to search/change all error message
3: Easy to modify function's internal for debug or other requirement,
   for example:
   print function/linenumber in error()
   dumpstack in error()
   add some trace for some style of message
   add support for -v, -vv, ...
   support for locales
   custom output functions
   support some special device/tty

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 utils.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

Comments

David Sterba Sept. 25, 2015, 10:50 a.m. UTC | #1
On Wed, Sep 16, 2015 at 05:40:46PM +0800, Zhao Lei wrote:
> +static inline void __veprintf(const char *prefix, const char *format,
> +			      va_list ap)
> +{
> +	if (prefix)
> +		fprintf(stderr, "%s", prefix);
> +	vfprintf(stderr, format, ap);

I'm not sure we need this helper. All it does it prints a fixed string,
we can simply add fputs("prefix", stderr) into warning/error functions.

> +static inline int warning_on(int condition, const char *fmt, ...)
> +{
> +	if (!condition)
> +		return 0;
> +
> +	va_list args;

Please do not put declaration(s) after statements.

> +static inline int error_on(int condition, const char *fmt, ...)
> +{
> +	if (!condition)
> +		return 0;
> +
> +	va_list args;

dtto

> +
> +	va_start(args, fmt);
> +	__veprintf("ERROR: ", fmt, args);
> +	va_end(args);
> +
> +	return 1;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhaolei Sept. 28, 2015, 2:07 p.m. UTC | #2
Hi, David Sterba

Thanks for review.

> -----Original Message-----
> From: David Sterba [mailto:dsterba@suse.cz]
> Sent: Friday, September 25, 2015 6:50 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: linux-btrfs@vger.kernel.org; Qu Wenruo <quwenruo@cn.fujitsu.com>
> Subject: Re: [PATCH v2 1/2] btrfs-progs: Introduce warning and error for
> common use
> 
> On Wed, Sep 16, 2015 at 05:40:46PM +0800, Zhao Lei wrote:
> > +static inline void __veprintf(const char *prefix, const char *format,
> > +			      va_list ap)
> > +{
> > +	if (prefix)
> > +		fprintf(stderr, "%s", prefix);
> > +	vfprintf(stderr, format, ap);
> 
> I'm not sure we need this helper. All it does it prints a fixed string, we can
> simply add fputs("prefix", stderr) into warning/error functions.
> 
This wrapper is to support locale in future:
Make all message output by one function, then we can put locale code into it
simply.

But now it is hard to introduce locale, because user-land tools
changes output frequently, we need to wait the output about fixed.

So I removed __veprintf() in v3, we can modify it when we need.
 
> > +static inline int warning_on(int condition, const char *fmt, ...) {
> > +	if (!condition)
> > +		return 0;
> > +
> > +	va_list args;
> 
> Please do not put declaration(s) after statements.
> 
Fixed in v3.

> > +static inline int error_on(int condition, const char *fmt, ...) {
> > +	if (!condition)
> > +		return 0;
> > +
> > +	va_list args;
> 
> dtto
> 
Fixed in v3.

Thanks
Zhaolei

> > +
> > +	va_start(args, fmt);
> > +	__veprintf("ERROR: ", fmt, args);
> > +	va_end(args);
> > +
> > +	return 1;
> > +}

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/utils.h b/utils.h
index dce0a47..c63e372 100644
--- a/utils.h
+++ b/utils.h
@@ -22,6 +22,7 @@ 
 #include <sys/stat.h>
 #include "ctree.h"
 #include <dirent.h>
+#include <stdarg.h>
 
 #define BTRFS_MKFS_SYSTEM_GROUP_SIZE (4 * 1024 * 1024)
 #define BTRFS_MKFS_SMALL_VOLUME_SIZE (1024 * 1024 * 1024)
@@ -269,4 +270,58 @@  const char *get_argv0_buf(void);
 
 unsigned int get_unit_mode_from_arg(int *argc, char *argv[], int df_mode);
 
+static inline void __veprintf(const char *prefix, const char *format,
+			      va_list ap)
+{
+	if (prefix)
+		fprintf(stderr, "%s", prefix);
+	vfprintf(stderr, format, ap);
+}
+
+static inline void warning(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	__veprintf("WARNING: ", fmt, args);
+	va_end(args);
+}
+
+static inline void error(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	__veprintf("ERROR: ", fmt, args);
+	va_end(args);
+}
+
+static inline int warning_on(int condition, const char *fmt, ...)
+{
+	if (!condition)
+		return 0;
+
+	va_list args;
+
+	va_start(args, fmt);
+	__veprintf("WARNING: ", fmt, args);
+	va_end(args);
+
+	return 1;
+}
+
+static inline int error_on(int condition, const char *fmt, ...)
+{
+	if (!condition)
+		return 0;
+
+	va_list args;
+
+	va_start(args, fmt);
+	__veprintf("ERROR: ", fmt, args);
+	va_end(args);
+
+	return 1;
+}
+
 #endif