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

[Qemu-devel] [PATCH v2 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.

Since v1:
- Fixed checkpatch errors (patchew)
- Added Stefano R-b

There are still checkpatch warnings (due to 8c06fbdf36b) for using the
Doxygen '/**' comment opening:

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

v1: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg05927.html

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        | 262 +++++++++++++++++++++++++++++++++++
 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, 271 insertions(+), 215 deletions(-)

-- 
2.17.2

[Qemu-devel] [PATCH v2 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 v2 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.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: Fixed checkpatch warnings (tabs)
---
 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      | 25 +++++++++++++++++++++++++
 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, 33 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..644f2d75bd 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -3,6 +3,31 @@
 
 #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 v2 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 644f2d75bd..f41b00ad37 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -47,6 +47,7 @@
  *    bytes and then add a NUL
  */
 void pstrcpy(char *buf, int buf_size, const char *str);
+
 /**
  * strpadcpy:
  * @buf: buffer to copy string into
@@ -60,6 +61,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
@@ -77,6 +79,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
@@ -94,6 +97,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
@@ -110,6 +114,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
@@ -126,6 +131,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
@@ -147,6 +153,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)
 {
@@ -155,27 +171,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 v2 1/3] util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
> 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

s/sens/sense/ s/than/as/

> the opposite string to size functions: qemu_strtosz*().
> Note than this function is already implemented in util/cutils.c.

s/than/that/

> 
> 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>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
> 
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

> +++ b/include/qemu/cutils.h
> @@ -3,6 +3,31 @@
>  
>  #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.

I would also mention that these macros are ONLY intended for use with
char arguments (as they CANNOT handle EOF); if you are doing int c =
getchar() or similar, you still want to call the original ctype macro.

Otherwise, the move and added comments makes sense to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
> 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(-)

I find documentation in .c files slightly easier to use (you can then
read the code right below to see if the documentation is still
accurate); but as we had an inconsistent mix, I'm also okay with your
patch consolidating all the documentation to one of the two files,
rather than the bad mix of half-and-half.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3539 bytes --]

On Fri, Jan 04, 2019 at 07:12:06PM +0100, Philippe Mathieu-Daudé wrote:
> 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>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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" };

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 7493 bytes --]

On Fri, Jan 04, 2019 at 07:12:07PM +0100, Philippe Mathieu-Daudé wrote:
> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
> 
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v2: Fixed checkpatch warnings (tabs)
> ---
>  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      | 25 +++++++++++++++++++++++++
>  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, 33 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..644f2d75bd 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -3,6 +3,31 @@
>  
>  #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"
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 21677 bytes --]

On Fri, Jan 04, 2019 at 07:12:08PM +0100, Philippe Mathieu-Daudé wrote:
> 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>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  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 644f2d75bd..f41b00ad37 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -47,6 +47,7 @@
>   *    bytes and then add a NUL
>   */
>  void pstrcpy(char *buf, int buf_size, const char *str);
> +
>  /**
>   * strpadcpy:
>   * @buf: buffer to copy string into
> @@ -60,6 +61,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
> @@ -77,6 +79,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
> @@ -94,6 +97,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
> @@ -110,6 +114,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
> @@ -126,6 +131,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
> @@ -147,6 +153,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)
>  {
> @@ -155,27 +171,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;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Stefano Garzarella]

On Fri, Jan 4, 2019 at 7:12 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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(-)

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

>
> 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
>

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

[Stefano Garzarella]

On Fri, Jan 4, 2019 at 7:13 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> 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(-)

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

>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index 644f2d75bd..f41b00ad37 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -47,6 +47,7 @@
>   *    bytes and then add a NUL
>   */
>  void pstrcpy(char *buf, int buf_size, const char *str);
> +
>  /**
>   * strpadcpy:
>   * @buf: buffer to copy string into
> @@ -60,6 +61,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
> @@ -77,6 +79,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
> @@ -94,6 +97,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
> @@ -110,6 +114,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
> @@ -126,6 +131,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
> @@ -147,6 +153,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)
>  {
> @@ -155,27 +171,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 v2 3/3] util/cutils: Move function documentations to the header

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
>> 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(-)
>
> I find documentation in .c files slightly easier to use (you can then
> read the code right below to see if the documentation is still
> accurate);

This is the kicker for me.  I try to cultivate a healthy suspicion of
comments in general, x10 for comments acting at a distance.

>            but as we had an inconsistent mix, I'm also okay with your
> patch consolidating all the documentation to one of the two files,
> rather than the bad mix of half-and-half.

Concur.

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

[Cornelia Huck]

On Fri,  4 Jan 2019 19:12:06 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> 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(-)

With the patch description fixed (as noted by Eric):

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

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

[Cornelia Huck]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On Fri, 4 Jan 2019 14:15:49 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
> > Introduced in cd390083ad1, these macros don't need to be in
> > a generic header.
> > Add documentation to justify their use.
> > 
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---  
> 
> > +++ b/include/qemu/cutils.h
> > @@ -3,6 +3,31 @@
> >  
> >  #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.  
> 
> I would also mention that these macros are ONLY intended for use with
> char arguments (as they CANNOT handle EOF); if you are doing int c =
> getchar() or similar, you still want to call the original ctype macro.
> 
> Otherwise, the move and added comments makes sense to me.
> 

Agreed on both counts.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Cornelia Huck]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

On Fri, 4 Jan 2019 14:17:14 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 1/4/19 12:12 PM, Philippe Mathieu-Daudé wrote:
> > 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(-)  
> 
> I find documentation in .c files slightly easier to use (you can then
> read the code right below to see if the documentation is still
> accurate); but as we had an inconsistent mix, I'm also okay with your
> patch consolidating all the documentation to one of the two files,
> rather than the bad mix of half-and-half.
> 

Another vote for documentation in .c files: If I use tags, I jump to
the implementation, not the declaration.

But also +1 for consistency.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Laurent Vivier]

On 04/01/2019 19:12, Philippe Mathieu-Daudé wrote:
> 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" };
> 

Applied to my trivial-patches branch with updated patch description.

Thanks,
Laurent

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

[Laurent Vivier]

On 04/01/2019 19:12, Philippe Mathieu-Daudé wrote:
> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
> 
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: Fixed checkpatch warnings (tabs)
> ---
>  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      | 25 +++++++++++++++++++++++++
>  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, 33 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..644f2d75bd 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -3,6 +3,31 @@
>  
>  #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"
>  
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

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

[Laurent Vivier]

On 04/01/2019 19:12, Philippe Mathieu-Daudé wrote:
> 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 644f2d75bd..f41b00ad37 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -47,6 +47,7 @@
>   *    bytes and then add a NUL
>   */
>  void pstrcpy(char *buf, int buf_size, const char *str);
> +
>  /**
>   * strpadcpy:
>   * @buf: buffer to copy string into
> @@ -60,6 +61,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
> @@ -77,6 +79,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
> @@ -94,6 +97,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
> @@ -110,6 +114,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
> @@ -126,6 +131,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
> @@ -147,6 +153,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)
>  {
> @@ -155,27 +171,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;
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

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

[Laurent Vivier]

On 30/01/2019 11:24, Laurent Vivier wrote:
> On 04/01/2019 19:12, Philippe Mathieu-Daudé wrote:
>> Introduced in cd390083ad1, these macros don't need to be in
>> a generic header.
>> Add documentation to justify their use.
>>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> v2: Fixed checkpatch warnings (tabs)
>> ---
>>  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      | 25 +++++++++++++++++++++++++
>>  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, 33 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..644f2d75bd 100644
>> --- a/include/qemu/cutils.h
>> +++ b/include/qemu/cutils.h
>> @@ -3,6 +3,31 @@
>>  
>>  #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"
>>  
>>
> 
> Applied to my trivial-patches branch.

This patch breaks build:

  CC      riscv32-softmmu/target/riscv/cpu.o
.../qemu/target/riscv/cpu.c: In function 'riscv_isa_string':
.../qemu/target/riscv/cpu.c:378:20: error: implicit declaration of function 'qemu_tolower'; did you mean 'qemu_irq_lower'? [-Werror=implicit-function-declaration]
             *p++ = qemu_tolower(riscv_exts[i]);
                    ^~~~~~~~~~~~
                    qemu_irq_lower
.../qemu/target/riscv/cpu.c:378:20: error: nested extern declaration of 'qemu_tolower' [-Werror=nested-externs]
cc1: all warnings being treated as errors

I remove the series from the trivial-patches branch.

Thanks,
Laurent