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

[Qemu-devel] [PATCH v3 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 v2:
- Fixed RISC-V (Laurent)

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

$ git backport-diff -u v2
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/3:[----] [--] 'util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"'
002/3:[0001] [FC] 'util/cutils: Move ctype macros to "cutils.h"'
003/3:[----] [--] 'util/cutils: Move function documentations to the header'

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00629.html
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 +
 target/riscv/cpu.c           |   1 +
 ui/keymaps.c                 |   1 +
 util/cutils.c                | 191 -------------------------
 util/id.c                    |   2 +-
 util/readline.c              |   1 -
 15 files changed, 272 insertions(+), 215 deletions(-)

-- 
2.20.1

[Qemu-riscv] [PATCH v3 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 v2:
- Fixed RISC-V (Laurent)

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

$ git backport-diff -u v2
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/3:[----] [--] 'util/cutils: Move size_to_str() from "qemu-common.h" to "cutils.h"'
002/3:[0001] [FC] 'util/cutils: Move ctype macros to "cutils.h"'
003/3:[----] [--] 'util/cutils: Move function documentations to the header'

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg00629.html
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 +
 target/riscv/cpu.c           |   1 +
 ui/keymaps.c                 |   1 +
 util/cutils.c                | 191 -------------------------
 util/id.c                    |   2 +-
 util/readline.c              |   1 -
 15 files changed, 272 insertions(+), 215 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v3 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 sense to find this function in the same header as
the opposite string to size functions: qemu_strtosz*().
Note that 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: Eric Blake <eblake@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@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.20.1

[Qemu-riscv] [PATCH v3 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 sense to find this function in the same header as
the opposite string to size functions: qemu_strtosz*().
Note that 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: Eric Blake <eblake@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@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.20.1

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

[Philippe Mathieu-Daudé]

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

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@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 +
 target/riscv/cpu.c         |  1 +
 ui/keymaps.c               |  1 +
 util/id.c                  |  2 +-
 util/readline.c            |  1 -
 13 files changed, 34 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 5da1439a8b..f36006bfce 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 811fdf913d..3ef42dfaf9 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 d8eb210c0c..cad019d4cc 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 04deec8030..173177081b 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/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d7e5302f..77fb2372b9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/cutils.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "qapi/error.h"
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 6e44f738ed..a672fe3dd3 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.20.1

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

[Philippe Mathieu-Daudé]

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

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@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 +
 target/riscv/cpu.c         |  1 +
 ui/keymaps.c               |  1 +
 util/id.c                  |  2 +-
 util/readline.c            |  1 -
 13 files changed, 34 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 5da1439a8b..f36006bfce 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 811fdf913d..3ef42dfaf9 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 d8eb210c0c..cad019d4cc 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 04deec8030..173177081b 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/target/riscv/cpu.c b/target/riscv/cpu.c
index 28d7e5302f..77fb2372b9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "qemu/cutils.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "qapi/error.h"
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 6e44f738ed..a672fe3dd3 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.20.1

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

[Cornelia Huck]

On Mon,  4 Feb 2019 22:12:03 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Stefano Garzarella <sgarzare@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 +
>  target/riscv/cpu.c         |  1 +
>  ui/keymaps.c               |  1 +
>  util/id.c                  |  2 +-
>  util/readline.c            |  1 -
>  13 files changed, 34 insertions(+), 22 deletions(-)

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

I'd add

"Note that these macros are intended for use with char arguments only,
they cannot handle EOF."

> + */
> +#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

With that added,

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

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

[Cornelia Huck]

On Mon,  4 Feb 2019 22:12:03 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Introduced in cd390083ad1, these macros don't need to be in
> a generic header.
> Add documentation to justify their use.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Stefano Garzarella <sgarzare@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 +
>  target/riscv/cpu.c         |  1 +
>  ui/keymaps.c               |  1 +
>  util/id.c                  |  2 +-
>  util/readline.c            |  1 -
>  13 files changed, 34 insertions(+), 22 deletions(-)

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

I'd add

"Note that these macros are intended for use with char arguments only,
they cannot handle EOF."

> + */
> +#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

With that added,

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

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

[Daniel P. Berrangé]

On Tue, Feb 05, 2019 at 11:32:19AM +0100, Cornelia Huck wrote:
> On Mon,  4 Feb 2019 22:12:03 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
> > Introduced in cd390083ad1, these macros don't need to be in
> > a generic header.
> > Add documentation to justify their use.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Reviewed-by: Stefano Garzarella <sgarzare@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 +
> >  target/riscv/cpu.c         |  1 +
> >  ui/keymaps.c               |  1 +
> >  util/id.c                  |  2 +-
> >  util/readline.c            |  1 -
> >  13 files changed, 34 insertions(+), 22 deletions(-)
> 
> > 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.
> 
> I'd add
> 
> "Note that these macros are intended for use with char arguments only,
> they cannot handle EOF."
> 
> > + */
> > +#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))

Not a problem with your patch, since this is just code movement, but
I would note that some (many?) uses of these functions in QEMU are likely
wrong / bad. These functions all check according to the current locale
rules, while IME most developers assume these APIs are following ASCII
match rules. IOW, these may well be allowing more characters through
than expected.

We might want to consider whether some callers need switching to use
the glib provided APIs that explicitly match in ASCII rules:

  https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-ascii-isalnum

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Daniel P. Berrangé]

On Tue, Feb 05, 2019 at 11:32:19AM +0100, Cornelia Huck wrote:
> On Mon,  4 Feb 2019 22:12:03 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
> > Introduced in cd390083ad1, these macros don't need to be in
> > a generic header.
> > Add documentation to justify their use.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Reviewed-by: Stefano Garzarella <sgarzare@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 +
> >  target/riscv/cpu.c         |  1 +
> >  ui/keymaps.c               |  1 +
> >  util/id.c                  |  2 +-
> >  util/readline.c            |  1 -
> >  13 files changed, 34 insertions(+), 22 deletions(-)
> 
> > 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.
> 
> I'd add
> 
> "Note that these macros are intended for use with char arguments only,
> they cannot handle EOF."
> 
> > + */
> > +#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))

Not a problem with your patch, since this is just code movement, but
I would note that some (many?) uses of these functions in QEMU are likely
wrong / bad. These functions all check according to the current locale
rules, while IME most developers assume these APIs are following ASCII
match rules. IOW, these may well be allowing more characters through
than expected.

We might want to consider whether some callers need switching to use
the glib provided APIs that explicitly match in ASCII rules:

  https://developer.gnome.org/glib/stable/glib-String-Utility-Functions.html#g-ascii-isalnum

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v3 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>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Stefano Garzarella <sgarzare@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.20.1

[Qemu-riscv] [PATCH v3 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>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Stefano Garzarella <sgarzare@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.20.1

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

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

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

We do?

There are two justifiable function comment placement styles: (1) next to
definition, and (2) next to declaration if it's in a header, else next
to definition.

The rationale for the latter is having the headers do double-duty as
interface documentation.

The rationale for the former is consistently placing the function
comments close to the code gives them a fighting chance to actually
match the code.

I'm in the "next to definition" camp.  If you want concise interface
specification, use something like Sphinx.

QEMU code is, as so often, in all camps.

>                         move the comments in cutils.h.

We document some cutils functions next to their definition, and some
next to their declaration.  The inconsistency is ugly, and your patch
fixes it.  I'd fix it in the other direction.

Even if we decide to fix this one in this direction, I object to the
sweeping generalization in the commit message :)

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

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

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

We do?

There are two justifiable function comment placement styles: (1) next to
definition, and (2) next to declaration if it's in a header, else next
to definition.

The rationale for the latter is having the headers do double-duty as
interface documentation.

The rationale for the former is consistently placing the function
comments close to the code gives them a fighting chance to actually
match the code.

I'm in the "next to definition" camp.  If you want concise interface
specification, use something like Sphinx.

QEMU code is, as so often, in all camps.

>                         move the comments in cutils.h.

We document some cutils functions next to their definition, and some
next to their declaration.  The inconsistency is ugly, and your patch
fixes it.  I'd fix it in the other direction.

Even if we decide to fix this one in this direction, I object to the
sweeping generalization in the commit message :)

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

[Peter Maydell]

On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
> There are two justifiable function comment placement styles: (1) next to
> definition, and (2) next to declaration if it's in a header, else next
> to definition.
>
> The rationale for the latter is having the headers do double-duty as
> interface documentation.
>
> The rationale for the former is consistently placing the function
> comments close to the code gives them a fighting chance to actually
> match the code.
>
> I'm in the "next to definition" camp.  If you want concise interface
> specification, use something like Sphinx.

I'm in the "next to declaration" camp. I don't want to have
to wade into your implementation to find out what it does:
document it for me in the interface, please.

My standard for patches I review is "if this is adding a new
function prototype in a header file, add a doc comment".

thanks
-- PMM

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

[Peter Maydell]

On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
> There are two justifiable function comment placement styles: (1) next to
> definition, and (2) next to declaration if it's in a header, else next
> to definition.
>
> The rationale for the latter is having the headers do double-duty as
> interface documentation.
>
> The rationale for the former is consistently placing the function
> comments close to the code gives them a fighting chance to actually
> match the code.
>
> I'm in the "next to definition" camp.  If you want concise interface
> specification, use something like Sphinx.

I'm in the "next to declaration" camp. I don't want to have
to wade into your implementation to find out what it does:
document it for me in the interface, please.

My standard for patches I review is "if this is adding a new
function prototype in a header file, add a doc comment".

thanks
-- PMM

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

[Daniel P. Berrangé]

On Tue, Feb 05, 2019 at 10:56:51AM +0000, Peter Maydell wrote:
> On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
> > There are two justifiable function comment placement styles: (1) next to
> > definition, and (2) next to declaration if it's in a header, else next
> > to definition.
> >
> > The rationale for the latter is having the headers do double-duty as
> > interface documentation.
> >
> > The rationale for the former is consistently placing the function
> > comments close to the code gives them a fighting chance to actually
> > match the code.
> >
> > I'm in the "next to definition" camp.  If you want concise interface
> > specification, use something like Sphinx.
> 
> I'm in the "next to declaration" camp. I don't want to have
> to wade into your implementation to find out what it does:
> document it for me in the interface, please.

I'm already looking at the header file to identify the function signature,
having the explanation / docs at the same place is desirable, especially
when the C file name is completely different from the header file name
forcing me to go grepping the code base to find where it is implemented.
The .c files are already volumous in size so not amenable to browsing
for the purpose of reading the docs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Daniel P. Berrangé]

On Tue, Feb 05, 2019 at 10:56:51AM +0000, Peter Maydell wrote:
> On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
> > There are two justifiable function comment placement styles: (1) next to
> > definition, and (2) next to declaration if it's in a header, else next
> > to definition.
> >
> > The rationale for the latter is having the headers do double-duty as
> > interface documentation.
> >
> > The rationale for the former is consistently placing the function
> > comments close to the code gives them a fighting chance to actually
> > match the code.
> >
> > I'm in the "next to definition" camp.  If you want concise interface
> > specification, use something like Sphinx.
> 
> I'm in the "next to declaration" camp. I don't want to have
> to wade into your implementation to find out what it does:
> document it for me in the interface, please.

I'm already looking at the header file to identify the function signature,
having the explanation / docs at the same place is desirable, especially
when the C file name is completely different from the header file name
forcing me to go grepping the code base to find where it is implemented.
The .c files are already volumous in size so not amenable to browsing
for the purpose of reading the docs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Feb 05, 2019 at 10:56:51AM +0000, Peter Maydell wrote:
>> On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
>> > There are two justifiable function comment placement styles: (1) next to
>> > definition, and (2) next to declaration if it's in a header, else next
>> > to definition.
>> >
>> > The rationale for the latter is having the headers do double-duty as
>> > interface documentation.
>> >
>> > The rationale for the former is consistently placing the function
>> > comments close to the code gives them a fighting chance to actually
>> > match the code.
>> >
>> > I'm in the "next to definition" camp.  If you want concise interface
>> > specification, use something like Sphinx.
>> 
>> I'm in the "next to declaration" camp. I don't want to have
>> to wade into your implementation to find out what it does:
>> document it for me in the interface, please.
>
> I'm already looking at the header file to identify the function signature,
> having the explanation / docs at the same place is desirable, especially
> when the C file name is completely different from the header file name
> forcing me to go grepping the code base to find where it is implemented.

If going to a definition of takes you more than a few keystrokes, your
editor is grossly deficient.

> The .c files are already volumous in size so not amenable to browsing
> for the purpose of reading the docs.
>
> Regards,
> Daniel

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Feb 05, 2019 at 10:56:51AM +0000, Peter Maydell wrote:
>> On Tue, 5 Feb 2019 at 06:44, Markus Armbruster <armbru@redhat.com> wrote:
>> > There are two justifiable function comment placement styles: (1) next to
>> > definition, and (2) next to declaration if it's in a header, else next
>> > to definition.
>> >
>> > The rationale for the latter is having the headers do double-duty as
>> > interface documentation.
>> >
>> > The rationale for the former is consistently placing the function
>> > comments close to the code gives them a fighting chance to actually
>> > match the code.
>> >
>> > I'm in the "next to definition" camp.  If you want concise interface
>> > specification, use something like Sphinx.
>> 
>> I'm in the "next to declaration" camp. I don't want to have
>> to wade into your implementation to find out what it does:
>> document it for me in the interface, please.
>
> I'm already looking at the header file to identify the function signature,
> having the explanation / docs at the same place is desirable, especially
> when the C file name is completely different from the header file name
> forcing me to go grepping the code base to find where it is implemented.

If going to a definition of takes you more than a few keystrokes, your
editor is grossly deficient.

> The .c files are already volumous in size so not amenable to browsing
> for the purpose of reading the docs.
>
> Regards,
> Daniel