[Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[Philippe Mathieu-Daudé]

This series is a fairly trivial cleanup of "cutils.h"
(size_to_str() and ctype macros moved into it), and
some documentation improvements.

Regards,

Phil.

Philippe Mathieu-Daudé (3):
  util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"
  util/cutils: Move ctype macros to "cutils.h"
  util/cutils: Move function documentations to the header

 hw/core/bus.c                |   2 +-
 hw/core/qdev-properties.c    |   1 +
 hw/s390x/s390-virtio-ccw.c   |   1 +
 hw/scsi/scsi-generic.c       |   2 +-
 include/qemu-common.h        |  17 ---
 include/qemu/cutils.h        | 261 +++++++++++++++++++++++++++++++++++
 qapi/qapi-util.c             |   2 +-
 qapi/string-output-visitor.c |   2 +-
 qobject/json-parser.c        |   1 -
 target/ppc/monitor.c         |   1 +
 ui/keymaps.c                 |   1 +
 util/cutils.c                | 191 -------------------------
 util/id.c                    |   2 +-
 util/readline.c              |   1 -
 14 files changed, 270 insertions(+), 215 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH 1/3] util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"

[Philippe Mathieu-Daudé]

The size_to_str() function doesn't need to be in a generic header.

It makes also sens to find this function in the same header than
the opposite string to size functions: qemu_strtosz*().
Note than this function is already implemented in util/cutils.c.

Since we introduce a new function in a header, we document it,
using the previous comment from the source file.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu-common.h        |  1 -
 include/qemu/cutils.h        | 13 +++++++++++++
 qapi/string-output-visitor.c |  2 +-
 util/cutils.c                |  6 ------
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index ed60ba251d..760527294f 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -153,7 +153,6 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
 int parse_debug_env(const char *name, int max, int initial);
 
 const char *qemu_ether_ntoa(const MACAddr *mac);
-char *size_to_str(uint64_t val);
 void page_size_init(void);
 
 /* returns non-zero if dump is in progress, otherwise zero is
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index d2dad3057c..9ee40470e3 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -157,6 +157,19 @@ int qemu_strtosz(const char *nptr, const char **end, uint64_t *result);
 int qemu_strtosz_MiB(const char *nptr, const char **end, uint64_t *result);
 int qemu_strtosz_metric(const char *nptr, const char **end, uint64_t *result);
 
+/**
+ * size_to_str:
+ *
+ * Return human readable string for size @val.
+ * Use IEC binary units like KiB, MiB, and so forth.
+ *
+ * @val: The value to format.
+ *       Can be anything that uint64_t allows (no more than "16 EiB").
+ *
+ * Caller is responsible for passing it to g_free().
+ */
+char *size_to_str(uint64_t val);
+
 /* used to print char* safely */
 #define STR_OR_NULL(str) ((str) ? (str) : "null")
 
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 7ab64468d9..edf268b373 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -11,9 +11,9 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu-common.h"
 #include "qapi/string-output-visitor.h"
 #include "qapi/visitor-impl.h"
+#include "qemu/cutils.h"
 #include "qemu/host-utils.h"
 #include <math.h>
 #include "qemu/range.h"
diff --git a/util/cutils.c b/util/cutils.c
index e098debdc0..a8a3a3ba3b 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -816,12 +816,6 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
     return ret;
 }
 
-/*
- * Return human readable string for size @val.
- * @val can be anything that uint64_t allows (no more than "16 EiB").
- * Use IEC binary units like KiB, MiB, and so forth.
- * Caller is responsible for passing it to g_free().
- */
 char *size_to_str(uint64_t val)
 {
     static const char *suffixes[] = { "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei" };
-- 
2.17.2

[Qemu-devel] [PATCH 2/3] util/cutils: Move ctype macros to "cutils.h"

[Philippe Mathieu-Daudé]

Introduced in cd390083ad1, these macros don't need to be in
a generic header.
Add documentation to justify their use.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/bus.c              |  2 +-
 hw/core/qdev-properties.c  |  1 +
 hw/s390x/s390-virtio-ccw.c |  1 +
 hw/scsi/scsi-generic.c     |  2 +-
 include/qemu-common.h      | 16 ----------------
 include/qemu/cutils.h      | 24 ++++++++++++++++++++++++
 qapi/qapi-util.c           |  2 +-
 qobject/json-parser.c      |  1 -
 target/ppc/monitor.c       |  1 +
 ui/keymaps.c               |  1 +
 util/id.c                  |  2 +-
 util/readline.c            |  1 -
 12 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/hw/core/bus.c b/hw/core/bus.c
index 4651f24486..dceb144075 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -18,7 +18,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu-common.h"
+#include "qemu/cutils.h"
 #include "hw/qdev.h"
 #include "qapi/error.h"
 
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 943dc2654b..3bdebac361 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1,4 +1,5 @@
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "net/net.h"
 #include "hw/qdev.h"
 #include "qapi/error.h"
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index fd9d0b0542..ed23bb7b3a 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "cpu.h"
 #include "hw/boards.h"
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 7237b4162e..86f65fd474 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -12,8 +12,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
-#include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "hw/scsi/scsi.h"
 #include "hw/scsi/emulation.h"
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 760527294f..ed43ae286d 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -33,22 +33,6 @@ int qemu_main(int argc, char **argv, char **envp);
 void qemu_get_timedate(struct tm *tm, int offset);
 int qemu_timedate_diff(struct tm *tm);
 
-#define qemu_isalnum(c)		isalnum((unsigned char)(c))
-#define qemu_isalpha(c)		isalpha((unsigned char)(c))
-#define qemu_iscntrl(c)		iscntrl((unsigned char)(c))
-#define qemu_isdigit(c)		isdigit((unsigned char)(c))
-#define qemu_isgraph(c)		isgraph((unsigned char)(c))
-#define qemu_islower(c)		islower((unsigned char)(c))
-#define qemu_isprint(c)		isprint((unsigned char)(c))
-#define qemu_ispunct(c)		ispunct((unsigned char)(c))
-#define qemu_isspace(c)		isspace((unsigned char)(c))
-#define qemu_isupper(c)		isupper((unsigned char)(c))
-#define qemu_isxdigit(c)	isxdigit((unsigned char)(c))
-#define qemu_tolower(c)		tolower((unsigned char)(c))
-#define qemu_toupper(c)		toupper((unsigned char)(c))
-#define qemu_isascii(c)		isascii((unsigned char)(c))
-#define qemu_toascii(c)		toascii((unsigned char)(c))
-
 void *qemu_oom_check(void *ptr);
 
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 9ee40470e3..cb06a5adab 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -3,6 +3,30 @@
 
 #include "qemu/fprintf-fn.h"
 
+/** unsigned ctype macros:
+ *
+ * The standards require that the argument for these functions
+ * is either EOF or a value that is representable in the type
+ * unsigned char. If the argument is of type char, it must be
+ * cast to unsigned char. This is what these macros do,
+ * avoiding 'signed to unsigned' conversion warnings.
+ */
+#define qemu_isalnum(c)		isalnum((unsigned char)(c))
+#define qemu_isalpha(c)		isalpha((unsigned char)(c))
+#define qemu_iscntrl(c)		iscntrl((unsigned char)(c))
+#define qemu_isdigit(c)		isdigit((unsigned char)(c))
+#define qemu_isgraph(c)		isgraph((unsigned char)(c))
+#define qemu_islower(c)		islower((unsigned char)(c))
+#define qemu_isprint(c)		isprint((unsigned char)(c))
+#define qemu_ispunct(c)		ispunct((unsigned char)(c))
+#define qemu_isspace(c)		isspace((unsigned char)(c))
+#define qemu_isupper(c)		isupper((unsigned char)(c))
+#define qemu_isxdigit(c)	isxdigit((unsigned char)(c))
+#define qemu_tolower(c)		tolower((unsigned char)(c))
+#define qemu_toupper(c)		toupper((unsigned char)(c))
+#define qemu_isascii(c)		isascii((unsigned char)(c))
+#define qemu_toascii(c)		toascii((unsigned char)(c))
+
 /**
  * pstrcpy:
  * @buf: buffer to copy string into
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index e9b266bb70..ea93ae05d9 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -11,8 +11,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
-#include "qemu-common.h"
 
 const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
 {
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 7a7ae9e8d1..06316f3a53 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -15,7 +15,6 @@
 #include "qemu/cutils.h"
 #include "qemu/unicode.h"
 #include "qapi/error.h"
-#include "qemu-common.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index 14915119fc..dbcb921231 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -22,6 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "cpu.h"
 #include "monitor/monitor.h"
 #include "monitor/hmp-target.h"
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 085889b555..00b52a6db3 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "keymaps.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
diff --git a/util/id.c b/util/id.c
index 6141352955..ca21a77522 100644
--- a/util/id.c
+++ b/util/id.c
@@ -11,7 +11,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu-common.h"
+#include "qemu/cutils.h"
 #include "qemu/id.h"
 
 bool id_wellformed(const char *id)
diff --git a/util/readline.c b/util/readline.c
index ec91ee0fea..f3d8b0698a 100644
--- a/util/readline.c
+++ b/util/readline.c
@@ -23,7 +23,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu-common.h"
 #include "qemu/readline.h"
 #include "qemu/cutils.h"
 
-- 
2.17.2

[Qemu-devel] [PATCH 3/3] util/cutils: Move function documentations to the header

[Philippe Mathieu-Daudé]

Many functions have documentation before the implementation in
cutils.c. Since we expect documentation around the prototype
declaration in headers, move the comments in cutils.h.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qemu/cutils.h | 224 ++++++++++++++++++++++++++++++++++++++++++
 util/cutils.c         | 185 ----------------------------------
 2 files changed, 224 insertions(+), 185 deletions(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index cb06a5adab..fa22152e07 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -46,6 +46,7 @@
  *    bytes and then add a NUL
  */
 void pstrcpy(char *buf, int buf_size, const char *str);
+
 /**
  * strpadcpy:
  * @buf: buffer to copy string into
@@ -59,6 +60,7 @@ void pstrcpy(char *buf, int buf_size, const char *str);
  * first @buf_size characters of @str, with no terminator.
  */
 void strpadcpy(char *buf, int buf_size, const char *str, char pad);
+
 /**
  * pstrcat:
  * @buf: buffer containing existing string
@@ -76,6 +78,7 @@ void strpadcpy(char *buf, int buf_size, const char *str, char pad);
  * Returns: @buf.
  */
 char *pstrcat(char *buf, int buf_size, const char *s);
+
 /**
  * strstart:
  * @str: string to test
@@ -93,6 +96,7 @@ char *pstrcat(char *buf, int buf_size, const char *s);
  * Returns: true if @str starts with prefix @val, false otherwise.
  */
 int strstart(const char *str, const char *val, const char **ptr);
+
 /**
  * stristart:
  * @str: string to test
@@ -109,6 +113,7 @@ int strstart(const char *str, const char *val, const char **ptr);
  *          false otherwise.
  */
 int stristart(const char *str, const char *val, const char **ptr);
+
 /**
  * qemu_strnlen:
  * @s: string
@@ -125,6 +130,7 @@ int stristart(const char *str, const char *val, const char **ptr);
  * Returns: length of @s in bytes, or @max_len, whichever is smaller.
  */
 int qemu_strnlen(const char *s, int max_len);
+
 /**
  * qemu_strsep:
  * @input: pointer to string to parse
@@ -146,6 +152,16 @@ int qemu_strnlen(const char *s, int max_len);
  * Returns: the pointer originally in @input.
  */
 char *qemu_strsep(char **input, const char *delim);
+
+/**
+ * qemu_strchrnul:
+ *
+ * @s: String to parse.
+ * @c: Character to find.
+ *
+ * Searches for the first occurrence of @c in @s, and returns a pointer
+ * to the trailing null byte if none was found.
+ */
 #ifdef HAVE_STRCHRNUL
 static inline const char *qemu_strchrnul(const char *s, int c)
 {
@@ -154,27 +170,235 @@ static inline const char *qemu_strchrnul(const char *s, int c)
 #else
 const char *qemu_strchrnul(const char *s, int c);
 #endif
+
 time_t mktimegm(struct tm *tm);
 int qemu_fdatasync(int fd);
 int fcntl_setfl(int fd, int flag);
 int qemu_parse_fd(const char *param);
+
+/**
+ * qemu_strtoi:
+ *
+ * Convert string @nptr to an integer, and store it in @result.
+ *
+ * This is a wrapper around strtol() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtol() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store INT_MAX in @result,
+ * and return -ERANGE.
+ *
+ * If the conversion underflows @result, store INT_MIN in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ */
 int qemu_strtoi(const char *nptr, const char **endptr, int base,
                 int *result);
+
+/**
+ * qemu_strtoui:
+ *
+ * Convert string @nptr to an unsigned integer, and store it in @result.
+ *
+ * This is a wrapper around strtoul() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtoul() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store UINT_MAX in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ *
+ * Note that a number with a leading minus sign gets converted without
+ * the minus sign, checked for overflow (see above), then negated (in
+ * @result's type).  This is exactly how strtoul() works.
+ */
 int qemu_strtoui(const char *nptr, const char **endptr, int base,
                  unsigned int *result);
+
+/**
+ * qemu_strtol:
+ *
+ * Convert string @nptr to a long integer, and store it in @result.
+ *
+ * This is a wrapper around strtol() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtol() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store LONG_MAX in @result,
+ * and return -ERANGE.
+ *
+ * If the conversion underflows @result, store LONG_MIN in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ */
 int qemu_strtol(const char *nptr, const char **endptr, int base,
                 long *result);
+
+/**
+ * qemu_strtoul:
+ *
+ * Convert string @nptr to an unsigned long, and store it in @result.
+ *
+ * This is a wrapper around strtoul() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtoul() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store ULONG_MAX in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ *
+ * Note that a number with a leading minus sign gets converted without
+ * the minus sign, checked for overflow (see above), then negated (in
+ * @result's type).  This is exactly how strtoul() works.
+ */
+
 int qemu_strtoul(const char *nptr, const char **endptr, int base,
                  unsigned long *result);
+
+/**
+ * qemu_strtoi64:
+ *
+ * Convert string @nptr to an int64_t.
+ *
+ * Works like qemu_strtol(), except it stores INT64_MAX on overflow,
+ * and INT_MIN on underflow.
+ */
 int qemu_strtoi64(const char *nptr, const char **endptr, int base,
                   int64_t *result);
+
+/**
+ * qemu_strtou64:
+ *
+ * Convert string @nptr to an uint64_t.
+ *
+ * Works like qemu_strtoul(), except it stores UINT64_MAX on overflow.
+ */
 int qemu_strtou64(const char *nptr, const char **endptr, int base,
                   uint64_t *result);
+
+/**
+ * qemu_strtod:
+ *
+ * Convert string @nptr to a double.
+ *
+ * This is a wrapper around strtod() that is harder to misuse.
+ * Semantics of @nptr and @endptr match strtod() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL. This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows, store +/-HUGE_VAL in @result, depending
+ * on the sign, and return -ERANGE.
+ *
+ * If the conversion underflows, store +/-0.0 in @result, depending on the
+ * sign, and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ */
 int qemu_strtod(const char *nptr, const char **endptr, double *result);
+
+/**
+ * qemu_strtod_finite:
+ *
+ * Convert string @nptr to a finite double.
+ *
+ * Works like qemu_strtod(), except that "NaN" and "inf" are rejected
+ * with -EINVAL and no conversion is performed.
+ */
 int qemu_strtod_finite(const char *nptr, const char **endptr, double *result);
 
+/**
+ * parse_uint:
+ *
+ * @s: String to parse
+ * @value: Destination for parsed integer value
+ * @endptr: Destination for pointer to first character not consumed
+ * @base: integer base, between 2 and 36 inclusive, or 0
+ *
+ * Parse unsigned integer
+ *
+ * Parsed syntax is like strtoull()'s: arbitrary whitespace, a single optional
+ * '+' or '-', an optional "0x" if @base is 0 or 16, one or more digits.
+ *
+ * If @s is null, or @base is invalid, or @s doesn't start with an
+ * integer in the syntax above, set *@value to 0, *@endptr to @s, and
+ * return -EINVAL.
+ *
+ * Set *@endptr to point right beyond the parsed integer (even if the integer
+ * overflows or is negative, all digits will be parsed and *@endptr will
+ * point right beyond them).
+ *
+ * If the integer is negative, set *@value to 0, and return -ERANGE.
+ *
+ * If the integer overflows unsigned long long, set *@value to
+ * ULLONG_MAX, and return -ERANGE.
+ *
+ * Else, set *@value to the parsed integer, and return 0.
+ */
 int parse_uint(const char *s, unsigned long long *value, char **endptr,
                int base);
+
+/**
+ * parse_uint_full:
+ *
+ * @s: String to parse
+ * @value: Destination for parsed integer value
+ * @base: integer base, between 2 and 36 inclusive, or 0
+ *
+ * Parse unsigned integer from entire string
+ *
+ * Have the same behavior of parse_uint(), but with an additional check
+ * for additional data after the parsed number. If extra characters are present
+ * after the parsed number, the function will return -EINVAL, and *@v will
+ * be set to 0.
+ */
 int parse_uint_full(const char *s, unsigned long long *value, int base);
 
 int qemu_strtosz(const char *nptr, const char **end, uint64_t *result);
diff --git a/util/cutils.c b/util/cutils.c
index a8a3a3ba3b..a4c8858712 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -296,30 +296,6 @@ static int check_strtox_error(const char *nptr, char *ep,
     return -libc_errno;
 }
 
-/**
- * Convert string @nptr to an integer, and store it in @result.
- *
- * This is a wrapper around strtol() that is harder to misuse.
- * Semantics of @nptr, @endptr, @base match strtol() with differences
- * noted below.
- *
- * @nptr may be null, and no conversion is performed then.
- *
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
- *
- * If @endptr is null, and the string isn't fully converted, return
- * -EINVAL.  This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
- *
- * If the conversion overflows @result, store INT_MAX in @result,
- * and return -ERANGE.
- *
- * If the conversion underflows @result, store INT_MIN in @result,
- * and return -ERANGE.
- *
- * Else store the converted value in @result, and return zero.
- */
 int qemu_strtoi(const char *nptr, const char **endptr, int base,
                 int *result)
 {
@@ -348,31 +324,6 @@ int qemu_strtoi(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to an unsigned integer, and store it in @result.
- *
- * This is a wrapper around strtoul() that is harder to misuse.
- * Semantics of @nptr, @endptr, @base match strtoul() with differences
- * noted below.
- *
- * @nptr may be null, and no conversion is performed then.
- *
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
- *
- * If @endptr is null, and the string isn't fully converted, return
- * -EINVAL.  This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
- *
- * If the conversion overflows @result, store UINT_MAX in @result,
- * and return -ERANGE.
- *
- * Else store the converted value in @result, and return zero.
- *
- * Note that a number with a leading minus sign gets converted without
- * the minus sign, checked for overflow (see above), then negated (in
- * @result's type).  This is exactly how strtoul() works.
- */
 int qemu_strtoui(const char *nptr, const char **endptr, int base,
                  unsigned int *result)
 {
@@ -407,30 +358,6 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to a long integer, and store it in @result.
- *
- * This is a wrapper around strtol() that is harder to misuse.
- * Semantics of @nptr, @endptr, @base match strtol() with differences
- * noted below.
- *
- * @nptr may be null, and no conversion is performed then.
- *
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
- *
- * If @endptr is null, and the string isn't fully converted, return
- * -EINVAL.  This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
- *
- * If the conversion overflows @result, store LONG_MAX in @result,
- * and return -ERANGE.
- *
- * If the conversion underflows @result, store LONG_MIN in @result,
- * and return -ERANGE.
- *
- * Else store the converted value in @result, and return zero.
- */
 int qemu_strtol(const char *nptr, const char **endptr, int base,
                 long *result)
 {
@@ -449,31 +376,6 @@ int qemu_strtol(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to an unsigned long, and store it in @result.
- *
- * This is a wrapper around strtoul() that is harder to misuse.
- * Semantics of @nptr, @endptr, @base match strtoul() with differences
- * noted below.
- *
- * @nptr may be null, and no conversion is performed then.
- *
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
- *
- * If @endptr is null, and the string isn't fully converted, return
- * -EINVAL.  This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
- *
- * If the conversion overflows @result, store ULONG_MAX in @result,
- * and return -ERANGE.
- *
- * Else store the converted value in @result, and return zero.
- *
- * Note that a number with a leading minus sign gets converted without
- * the minus sign, checked for overflow (see above), then negated (in
- * @result's type).  This is exactly how strtoul() works.
- */
 int qemu_strtoul(const char *nptr, const char **endptr, int base,
                  unsigned long *result)
 {
@@ -496,12 +398,6 @@ int qemu_strtoul(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to an int64_t.
- *
- * Works like qemu_strtol(), except it stores INT64_MAX on overflow,
- * and INT_MIN on underflow.
- */
 int qemu_strtoi64(const char *nptr, const char **endptr, int base,
                  int64_t *result)
 {
@@ -521,11 +417,6 @@ int qemu_strtoi64(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to an uint64_t.
- *
- * Works like qemu_strtoul(), except it stores UINT64_MAX on overflow.
- */
 int qemu_strtou64(const char *nptr, const char **endptr, int base,
                   uint64_t *result)
 {
@@ -549,30 +440,6 @@ int qemu_strtou64(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to a double.
-  *
- * This is a wrapper around strtod() that is harder to misuse.
- * Semantics of @nptr and @endptr match strtod() with differences
- * noted below.
- *
- * @nptr may be null, and no conversion is performed then.
- *
- * If no conversion is performed, store @nptr in *@endptr and return
- * -EINVAL.
- *
- * If @endptr is null, and the string isn't fully converted, return
- * -EINVAL. This is the case when the pointer that would be stored in
- * a non-null @endptr points to a character other than '\0'.
- *
- * If the conversion overflows, store +/-HUGE_VAL in @result, depending
- * on the sign, and return -ERANGE.
- *
- * If the conversion underflows, store +/-0.0 in @result, depending on the
- * sign, and return -ERANGE.
- *
- * Else store the converted value in @result, and return zero.
- */
 int qemu_strtod(const char *nptr, const char **endptr, double *result)
 {
     char *ep;
@@ -589,12 +456,6 @@ int qemu_strtod(const char *nptr, const char **endptr, double *result)
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
-/**
- * Convert string @nptr to a finite double.
- *
- * Works like qemu_strtod(), except that "NaN" and "inf" are rejected
- * with -EINVAL and no conversion is performed.
- */
 int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
 {
     double tmp;
@@ -614,10 +475,6 @@ int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
     return ret;
 }
 
-/**
- * Searches for the first occurrence of 'c' in 's', and returns a pointer
- * to the trailing null byte if none was found.
- */
 #ifndef HAVE_STRCHRNUL
 const char *qemu_strchrnul(const char *s, int c)
 {
@@ -629,34 +486,6 @@ const char *qemu_strchrnul(const char *s, int c)
 }
 #endif
 
-/**
- * parse_uint:
- *
- * @s: String to parse
- * @value: Destination for parsed integer value
- * @endptr: Destination for pointer to first character not consumed
- * @base: integer base, between 2 and 36 inclusive, or 0
- *
- * Parse unsigned integer
- *
- * Parsed syntax is like strtoull()'s: arbitrary whitespace, a single optional
- * '+' or '-', an optional "0x" if @base is 0 or 16, one or more digits.
- *
- * If @s is null, or @base is invalid, or @s doesn't start with an
- * integer in the syntax above, set *@value to 0, *@endptr to @s, and
- * return -EINVAL.
- *
- * Set *@endptr to point right beyond the parsed integer (even if the integer
- * overflows or is negative, all digits will be parsed and *@endptr will
- * point right beyond them).
- *
- * If the integer is negative, set *@value to 0, and return -ERANGE.
- *
- * If the integer overflows unsigned long long, set *@value to
- * ULLONG_MAX, and return -ERANGE.
- *
- * Else, set *@value to the parsed integer, and return 0.
- */
 int parse_uint(const char *s, unsigned long long *value, char **endptr,
                int base)
 {
@@ -698,20 +527,6 @@ out:
     return r;
 }
 
-/**
- * parse_uint_full:
- *
- * @s: String to parse
- * @value: Destination for parsed integer value
- * @base: integer base, between 2 and 36 inclusive, or 0
- *
- * Parse unsigned integer from entire string
- *
- * Have the same behavior of parse_uint(), but with an additional check
- * for additional data after the parsed number. If extra characters are present
- * after the parsed number, the function will return -EINVAL, and *@v will
- * be set to 0.
- */
 int parse_uint_full(const char *s, unsigned long long *value, int base)
 {
     char *endp;
-- 
2.17.2

Re: [Qemu-devel] [PATCH 2/3] util/cutils: Move ctype macros to "cutils.h"

[Stefano Garzarella]

On Wed, Dec 26, 2018 at 6:25 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/core/bus.c              |  2 +-
>  hw/core/qdev-properties.c  |  1 +
>  hw/s390x/s390-virtio-ccw.c |  1 +
>  hw/scsi/scsi-generic.c     |  2 +-
>  include/qemu-common.h      | 16 ----------------
>  include/qemu/cutils.h      | 24 ++++++++++++++++++++++++
>  qapi/qapi-util.c           |  2 +-
>  qobject/json-parser.c      |  1 -
>  target/ppc/monitor.c       |  1 +
>  ui/keymaps.c               |  1 +
>  util/id.c                  |  2 +-
>  util/readline.c            |  1 -
>  12 files changed, 32 insertions(+), 22 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index 4651f24486..dceb144075 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -18,7 +18,7 @@
>   */
>
>  #include "qemu/osdep.h"
> -#include "qemu-common.h"
> +#include "qemu/cutils.h"
>  #include "hw/qdev.h"
>  #include "qapi/error.h"
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 943dc2654b..3bdebac361 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1,4 +1,5 @@
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "net/net.h"
>  #include "hw/qdev.h"
>  #include "qapi/error.h"
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index fd9d0b0542..ed23bb7b3a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -11,6 +11,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
>  #include "hw/boards.h"
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 7237b4162e..86f65fd474 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -12,8 +12,8 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
> -#include "qemu-common.h"
>  #include "qemu/error-report.h"
>  #include "hw/scsi/scsi.h"
>  #include "hw/scsi/emulation.h"
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 760527294f..ed43ae286d 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -33,22 +33,6 @@ int qemu_main(int argc, char **argv, char **envp);
>  void qemu_get_timedate(struct tm *tm, int offset);
>  int qemu_timedate_diff(struct tm *tm);
>
> -#define qemu_isalnum(c)                isalnum((unsigned char)(c))
> -#define qemu_isalpha(c)                isalpha((unsigned char)(c))
> -#define qemu_iscntrl(c)                iscntrl((unsigned char)(c))
> -#define qemu_isdigit(c)                isdigit((unsigned char)(c))
> -#define qemu_isgraph(c)                isgraph((unsigned char)(c))
> -#define qemu_islower(c)                islower((unsigned char)(c))
> -#define qemu_isprint(c)                isprint((unsigned char)(c))
> -#define qemu_ispunct(c)                ispunct((unsigned char)(c))
> -#define qemu_isspace(c)                isspace((unsigned char)(c))
> -#define qemu_isupper(c)                isupper((unsigned char)(c))
> -#define qemu_isxdigit(c)       isxdigit((unsigned char)(c))
> -#define qemu_tolower(c)                tolower((unsigned char)(c))
> -#define qemu_toupper(c)                toupper((unsigned char)(c))
> -#define qemu_isascii(c)                isascii((unsigned char)(c))
> -#define qemu_toascii(c)                toascii((unsigned char)(c))
> -
>  void *qemu_oom_check(void *ptr);
>
>  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 9ee40470e3..cb06a5adab 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -3,6 +3,30 @@
>
>  #include "qemu/fprintf-fn.h"
>
> +/** unsigned ctype macros:
> + *
> + * The standards require that the argument for these functions
> + * is either EOF or a value that is representable in the type
> + * unsigned char. If the argument is of type char, it must be
> + * cast to unsigned char. This is what these macros do,
> + * avoiding 'signed to unsigned' conversion warnings.
> + */
> +#define qemu_isalnum(c)                isalnum((unsigned char)(c))
> +#define qemu_isalpha(c)                isalpha((unsigned char)(c))
> +#define qemu_iscntrl(c)                iscntrl((unsigned char)(c))
> +#define qemu_isdigit(c)                isdigit((unsigned char)(c))
> +#define qemu_isgraph(c)                isgraph((unsigned char)(c))
> +#define qemu_islower(c)                islower((unsigned char)(c))
> +#define qemu_isprint(c)                isprint((unsigned char)(c))
> +#define qemu_ispunct(c)                ispunct((unsigned char)(c))
> +#define qemu_isspace(c)                isspace((unsigned char)(c))
> +#define qemu_isupper(c)                isupper((unsigned char)(c))
> +#define qemu_isxdigit(c)       isxdigit((unsigned char)(c))
> +#define qemu_tolower(c)                tolower((unsigned char)(c))
> +#define qemu_toupper(c)                toupper((unsigned char)(c))
> +#define qemu_isascii(c)                isascii((unsigned char)(c))
> +#define qemu_toascii(c)                toascii((unsigned char)(c))
> +
>  /**
>   * pstrcpy:
>   * @buf: buffer to copy string into
> diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
> index e9b266bb70..ea93ae05d9 100644
> --- a/qapi/qapi-util.c
> +++ b/qapi/qapi-util.c
> @@ -11,8 +11,8 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
> -#include "qemu-common.h"
>
>  const char *qapi_enum_lookup(const QEnumLookup *lookup, int val)
>  {
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 7a7ae9e8d1..06316f3a53 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -15,7 +15,6 @@
>  #include "qemu/cutils.h"
>  #include "qemu/unicode.h"
>  #include "qapi/error.h"
> -#include "qemu-common.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qlist.h"
> diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
> index 14915119fc..dbcb921231 100644
> --- a/target/ppc/monitor.c
> +++ b/target/ppc/monitor.c
> @@ -22,6 +22,7 @@
>   * THE SOFTWARE.
>   */
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "cpu.h"
>  #include "monitor/monitor.h"
>  #include "monitor/hmp-target.h"
> diff --git a/ui/keymaps.c b/ui/keymaps.c
> index 085889b555..00b52a6db3 100644
> --- a/ui/keymaps.c
> +++ b/ui/keymaps.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "keymaps.h"
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
> diff --git a/util/id.c b/util/id.c
> index 6141352955..ca21a77522 100644
> --- a/util/id.c
> +++ b/util/id.c
> @@ -11,7 +11,7 @@
>   */
>
>  #include "qemu/osdep.h"
> -#include "qemu-common.h"
> +#include "qemu/cutils.h"
>  #include "qemu/id.h"
>
>  bool id_wellformed(const char *id)
> diff --git a/util/readline.c b/util/readline.c
> index ec91ee0fea..f3d8b0698a 100644
> --- a/util/readline.c
> +++ b/util/readline.c
> @@ -23,7 +23,6 @@
>   */
>
>  #include "qemu/osdep.h"
> -#include "qemu-common.h"
>  #include "qemu/readline.h"
>  #include "qemu/cutils.h"
>
> --
> 2.17.2
>
>


--
Stefano Garzarella
Red Hat

Re: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20181226171538.21984-1-philmd@redhat.com
Subject: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5ac365d util/cutils: Move function documentations to the header
1d4b496 util/cutils: Move ctype macros to "cutils.h"
70db713 util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"

=== OUTPUT BEGIN ===
Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/qemu/cutils.h:160:
+/**

total: 0 errors, 1 warnings, 48 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/3: util/cutils: Move ctype macros to "cutils.h"...
WARNING: Block comments use a leading /* on a separate line
#100: FILE: include/qemu/cutils.h:6:
+/** unsigned ctype macros:

ERROR: code indent should never use tabs
#108: FILE: include/qemu/cutils.h:14:
+#define qemu_isalnum(c)^I^Iisalnum((unsigned char)(c))$

ERROR: code indent should never use tabs
#109: FILE: include/qemu/cutils.h:15:
+#define qemu_isalpha(c)^I^Iisalpha((unsigned char)(c))$

ERROR: code indent should never use tabs
#110: FILE: include/qemu/cutils.h:16:
+#define qemu_iscntrl(c)^I^Iiscntrl((unsigned char)(c))$

ERROR: code indent should never use tabs
#111: FILE: include/qemu/cutils.h:17:
+#define qemu_isdigit(c)^I^Iisdigit((unsigned char)(c))$

ERROR: code indent should never use tabs
#112: FILE: include/qemu/cutils.h:18:
+#define qemu_isgraph(c)^I^Iisgraph((unsigned char)(c))$

ERROR: code indent should never use tabs
#113: FILE: include/qemu/cutils.h:19:
+#define qemu_islower(c)^I^Iislower((unsigned char)(c))$

ERROR: code indent should never use tabs
#114: FILE: include/qemu/cutils.h:20:
+#define qemu_isprint(c)^I^Iisprint((unsigned char)(c))$

ERROR: code indent should never use tabs
#115: FILE: include/qemu/cutils.h:21:
+#define qemu_ispunct(c)^I^Iispunct((unsigned char)(c))$

ERROR: code indent should never use tabs
#116: FILE: include/qemu/cutils.h:22:
+#define qemu_isspace(c)^I^Iisspace((unsigned char)(c))$

ERROR: code indent should never use tabs
#117: FILE: include/qemu/cutils.h:23:
+#define qemu_isupper(c)^I^Iisupper((unsigned char)(c))$

ERROR: code indent should never use tabs
#118: FILE: include/qemu/cutils.h:24:
+#define qemu_isxdigit(c)^Iisxdigit((unsigned char)(c))$

ERROR: code indent should never use tabs
#119: FILE: include/qemu/cutils.h:25:
+#define qemu_tolower(c)^I^Itolower((unsigned char)(c))$

ERROR: code indent should never use tabs
#120: FILE: include/qemu/cutils.h:26:
+#define qemu_toupper(c)^I^Itoupper((unsigned char)(c))$

ERROR: code indent should never use tabs
#121: FILE: include/qemu/cutils.h:27:
+#define qemu_isascii(c)^I^Iisascii((unsigned char)(c))$

ERROR: code indent should never use tabs
#122: FILE: include/qemu/cutils.h:28:
+#define qemu_toascii(c)^I^Itoascii((unsigned char)(c))$

total: 15 errors, 1 warnings, 126 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/3: util/cutils: Move function documentations to the header...
WARNING: Block comments use a leading /* on a separate line
#73: FILE: include/qemu/cutils.h:156:
+/**

WARNING: Block comments use a leading /* on a separate line
#95: FILE: include/qemu/cutils.h:179:
+/**

WARNING: Block comments use a leading /* on a separate line
#124: FILE: include/qemu/cutils.h:208:
+/**

WARNING: Block comments use a leading /* on a separate line
#154: FILE: include/qemu/cutils.h:238:
+/**

WARNING: Block comments use a leading /* on a separate line
#183: FILE: include/qemu/cutils.h:267:
+/**

WARNING: Block comments use a leading /* on a separate line
#214: FILE: include/qemu/cutils.h:298:
+/**

WARNING: Block comments use a leading /* on a separate line
#225: FILE: include/qemu/cutils.h:309:
+/**

WARNING: Block comments use a leading /* on a separate line
#235: FILE: include/qemu/cutils.h:319:
+/**

WARNING: Block comments use a leading /* on a separate line
#263: FILE: include/qemu/cutils.h:347:
+/**

WARNING: Block comments use a leading /* on a separate line
#273: FILE: include/qemu/cutils.h:357:
+/**

WARNING: Block comments use a leading /* on a separate line
#304: FILE: include/qemu/cutils.h:388:
+/**

total: 0 errors, 11 warnings, 544 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181226171538.21984-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[Philippe Mathieu-Daudé]

On 1/2/19 6:41 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/
[...]> === OUTPUT BEGIN ===
> Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"...
> WARNING: Block comments use a leading /* on a separate line
> #42: FILE: include/qemu/cutils.h:160:
> +/**

I believe this warning is incorrect, since we use the /** marking for
Doxygen generated documentation. The offending comment is:

  /**
   * size_to_str:
   *
   * Return human readable string for size @val.
   * Use IEC binary units like KiB, MiB, and so forth.
   *
   * @val: The value to format.
   *       Can be anything that uint64_t allows (no more than "16 EiB").
   *
   * Caller is responsible for passing it to g_free().
   */
   char *size_to_str(uint64_t val);

Am I missing something?

> Checking PATCH 2/3: util/cutils: Move ctype macros to "cutils.h"...
> WARNING: Block comments use a leading /* on a separate line
> #100: FILE: include/qemu/cutils.h:6:
> +/** unsigned ctype macros:
> 
> ERROR: code indent should never use tabs
> #108: FILE: include/qemu/cutils.h:14:
> +#define qemu_isalnum(c)^I^Iisalnum((unsigned char)(c))$

OK, this is code movement I forgot to fix :/

> Checking PATCH 3/3: util/cutils: Move function documentations to the header...
> WARNING: Block comments use a leading /* on a separate line
> #73: FILE: include/qemu/cutils.h:156:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #95: FILE: include/qemu/cutils.h:179:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #124: FILE: include/qemu/cutils.h:208:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #154: FILE: include/qemu/cutils.h:238:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #183: FILE: include/qemu/cutils.h:267:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #214: FILE: include/qemu/cutils.h:298:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #225: FILE: include/qemu/cutils.h:309:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #235: FILE: include/qemu/cutils.h:319:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #263: FILE: include/qemu/cutils.h:347:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #273: FILE: include/qemu/cutils.h:357:
> +/**
> 
> WARNING: Block comments use a leading /* on a separate line
> #304: FILE: include/qemu/cutils.h:388:
> +/**

Re: [Qemu-devel] [PATCH 0/3] cutils: Cleanup, improve documentation

[Philippe Mathieu-Daudé]

Cc'ing Markus and Thomas who reviewed commit 8c06fbdf36b.

On Thu, Jan 3, 2019 at 10:04 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
> On 1/2/19 6:41 PM, no-reply@patchew.org wrote:
> > Patchew URL: https://patchew.org/QEMU/20181226171538.21984-1-philmd@redhat.com/
> [...]> === OUTPUT BEGIN ===
> > Checking PATCH 1/3: util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"...
> > WARNING: Block comments use a leading /* on a separate line
> > #42: FILE: include/qemu/cutils.h:160:
> > +/**
>
> I believe this warning is incorrect, since we use the /** marking for
> Doxygen generated documentation. The offending comment is:
>
>   /**
>    * size_to_str:
>    *
>    * Return human readable string for size @val.
>    * Use IEC binary units like KiB, MiB, and so forth.
>    *
>    * @val: The value to format.
>    *       Can be anything that uint64_t allows (no more than "16 EiB").
>    *
>    * Caller is responsible for passing it to g_free().
>    */
>    char *size_to_str(uint64_t val);
>
> Am I missing something?

I had a quick look at scripts/checkpatch.pl:

  # Block comment styles

      # Block comments use /* on a line of its own
      if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&    #inline /*...*/
          $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** non-blank
          WARN("Block comments use a leading /* on a separate line\n"
. $herecurr);
      }

I am confused because the comment says it allow blank /**, which is
the case here.