Re: [coreboot] [PATCH] [libpayload] [5/7] strsep and string functions documentation

[Uwe Hermann]

On Fri, Sep 26, 2008 at 01:21:04AM +0200, Stefan Reinauer wrote:
> Add strsep (since strtok is considered obsolete)
> add a bunch of string function doxygen comments.
>
> Signed-off-by: Stefan Reinauer

With the cosmetic changes listed below this is

Acked-by: Uwe Hermann
> Index: include/libpayload.h
>
> --- include/libpayload.h	(revision 3600)
> +++ include/libpayload.h	(working copy)
> @@ -309,6 +324,7 @@
>  char *strchr(const char *s, int c);
>  char *strdup(const char *s);
>  char *strstr(const char *h, const char *n);
> +char *strsep(char **stringp, const char *delim);
>  /** @} */
>
> /**
> Index: libc/string.c
> ===================================================================
> --- libc/string.c	(revision 3600)
> +++ libc/string.c	(working copy)
> @@ -128,6 +128,14 @@
>    return 0;
>  }
>
> +/**
> Copy a string with a maximum length.
> @param d The destination memory.
> @param s The source string.
> @param n Copy at most n characters as length of the string.
> @return strncpy returns a pointer to the destination memory.

Make this "@return A pointer to the destination memory." please.
Doxygen already puts "Returns:" in its output, no need to repeat
that with "strncpy returns".
> */
>
> char *strncpy(char *d, const char *s, size_t n)
>  {
>    /* Use +1 to get the NUL terminator. */
> @@ -140,11 +148,26 @@
>    return d;
>  }
>
> +/**
> Copy a string.
> @param d The destination memory.
> @param s The source string.
> @return strcpy returns a pointer to the destination memory.

Ditto, drop "strcpy returns", capitalize to "A pointer"...
> */
>
> char *strcpy(char *d, const char *s)
>  {
>    return strncpy(d, s, strlen(s) + 1);
>  }
>
> +/**
> Concatenates two strings with a maximum length.
> @param d The destination string.
> @param s The source string.
> @param n The target string will have a length of n characters at most.
> @return strncat returns a pointer to the destination string.

Ditto.
> */
>
> char *strncat(char *d, const char *s, size_t n)
>  {
>    char *p = d + strlen(d);
> @@ -158,6 +181,14 @@
>    return d;
>  }
>
> +/**
> Find a character in a string.
> @param s The string.
> @param c The character.
> @return strchr returns a pointer to the first occurence of the character in
> the string, or NULL if the character was not encountered within the string.
> */

Ditto.
> char *strchr(const char *s, int c)
>  {
>    char *p = (char *)s;
> @@ -170,6 +201,12 @@
>    return NULL;
>  }
>
> +/**
> Duplicate a string.
> @param s The string.

"The string to duplicate." may be a bit more readable.
> @return strdup returns a pointer to the copy of the original string.

As above, drop "strdup returns", capitalize to "A pointer...".
> */
>
> char *strdup(const char *s)
>  {
>    int n = strlen(s);
> @@ -182,6 +219,14 @@
>    return p;
>  }
>
> +/**
> Find a substring within a string.
> @param h The haystack string.
> @param n The needle string (substring)

Missing fullstop.
> @return strstr returns a pointer to the first occurence of the substring in

See above.
> the string, or NULL if the substring was not encountered within the string.
> */
>
> char *strstr(const char *h, const char *n)
>  {
>    int hn = strlen(h);
> @@ -194,3 +239,35 @@
>
> return NULL;
>  }
>
> +/**
> Separate strings

Missing full stop.
> @param stringp reference of the string to separate

Ditto.
> @param delim string containing all delimiters

Ditto.
> @return token string.

This could be a bit more verbose, but it's ok for now.
Please capitalize the first letter, though.
> */
>
> +char *strsep(char **stringp, const char *delim)
> +{
> char *walk, *token;
> if (!stringp || !*stringp || !**stringp)
> return NULL;
> token = walk = *stringp;
> /* Walk, search for delimiters */
> while(*walk && !strchr(delim, *walk))
> walk++;
> if (*walk) {
> /* Null terminate */

Null -> NUL ?
> *walk = '\0';
> walk++;
> }
> *stringp = walk;
> return token;
>
> +}

Uwe.