[PATCH] scanner: Improve documentation for strtouint()

Yong Bakos junk at humanoriented.com
Sat Jul 9 02:38:13 UTC 2016


On Jul 8, 2016, at 4:51 PM, Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
> From: Bryce Harrington <bryce at bryceharrington.org>
> 
> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>

A definite improvement. However, a couple questions inline below.


> ---
> src/scanner.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/scanner.c b/src/scanner.c
> index 4708cae..a266da1 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -576,8 +576,17 @@ free_interface(struct interface *interface)
> 	free(interface);
> }
> 
> -/* convert string to unsigned integer,
> - * in the case of error, return -1 */
> +/** Convert string to unsigned integer

Should this be a standard comment block instead of a doc-comment?
Since doxygen will not generate public documentation for scanner.c,
and since this is an internal function, maybe this should just be a
regular comment block.


> + *
> + * Parses a non-negative base-10 number from the given string.  If the
> + * specified string is blank, contains non-numerical characters, is out
> + * of range, or results in a negative number, -1 is returned to indicate
> + * an error..

Nit: single period here.


> + *
> + * This routine does not modify errno, nor sets errno on error.
> + *

This feels a little misleading as written, since it does potentially
modify errno temporarily as an implementation detail. Perhaps "Upon error,
this routine doe not modify or set errno" would be a little more clear.

Other than that, this is

Reviewed-by: Yong Bakos <ybakos at humanoriented.com>

Regards,
yong


> + * \returns -1 on error, or a non-negative integer on success
> + */
> static int
> strtouint(const char *str)
> {
> -- 
> 1.9.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel



More information about the wayland-devel mailing list