Re: [Qemu-devel] [PATCH v3 3/3] util/cutils: Move function documentations to the header
Daniel P. Berrangé writes: > On Tue, Feb 05, 2019 at 10:56:51AM +, Peter Maydell wrote: >> On Tue, 5 Feb 2019 at 06:44, Markus Armbruster 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-devel] [PATCH v3 3/3] util/cutils: Move function documentations to the header
On Tue, Feb 05, 2019 at 10:56:51AM +, Peter Maydell wrote: > On Tue, 5 Feb 2019 at 06:44, Markus Armbruster 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
On Tue, 5 Feb 2019 at 06:44, Markus Armbruster 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
Philippe Mathieu-Daudé 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 :)
[Qemu-devel] [PATCH v3 3/3] util/cutils: Move function documentations to the header
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é
Reviewed-by: David Gibson
Reviewed-by: Stefano Garzarella
---
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
