[PATCH] scanner: Improve documentation for strtouint()

Bryce Harrington bryce at osg.samsung.com
Mon Jul 11 21:18:20 UTC 2016


On Fri, Jul 08, 2016 at 07:38:13PM -0700, Yong Bakos wrote:
> 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.

Alright.

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

Hmm, you're right it's a bit awkward.  I'll use your text for now, we'll
get another go at the phrasing if/when the routine is moved to being a
shared utility routine.

> Other than that, this is
> 
> Reviewed-by: Yong Bakos <ybakos at humanoriented.com>

Thanks, pushed with your suggested edits.

   1cda73f..c88ec7e  master -> master


> 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