Hi,
Kent R. Spillner wrote on Wed, Jun 04, 2014 at 10:01:12AM -0500:
> config(char *) contains a hand-rolled version of getlist(char *).
Indeed.
> The only difference is that the hand-rolled version includes a NULL
> check before the strcmp.
You misread the code. There is no NULL check for the string p.
The NULL check for tp is just the usual NULL check contained
in the FOREACH macro to stop at the end of the list.
> Replace this with a call to getlist(char *) instead,
That is correct and adds clarity IMHO.
> and move the NULL check there to protect other callers as well.
You are moving no NULL check but adding a completely new one.
That new NULL check seems pointless.
> I think we can probably drop the NULL check altogether because we
> already check p is not NULL as the loop condition in the for-loop.
Correct.
> However, since the original code had an explicit check I decided
> to preserve it.
As explained above, it had not.
> I'm ok dropping the second hunk of this diff
> if others agree it's unnecessary.
Any OKs to commit just the first chunk?
Ingo
Index: usr.bin/man/config.c
===================================================================
RCS file: /work/cvsroot/src/usr.bin/man/config.c,v
retrieving revision 1.9
diff -p -u -r1.9 config.c
--- usr.bin/man/config.c 27 Oct 2009 23:59:40 -0000 1.9
+++ usr.bin/man/config.c 4 Jun 2014 14:51:10 -0000
@@ -92,8 +92,7 @@ config(char *fname)
continue;
*t = '\0';
- for (tp = TAILQ_FIRST(&head); /* Find any matching tag. */
- tp != NULL && strcmp(p, tp->s); tp = TAILQ_NEXT(tp, q));
+ tp = getlist(p); /* Find any matching tag. */
if (tp == NULL) /* Create a new tag. */
tp = addlist(p);
This is not useful:
> @@ -147,6 +146,9 @@ TAG *
> getlist(char *name)
> {
> TAG *tp;
> +
> + if (name == NULL)
> + return (NULL);
>
> TAILQ_FOREACH(tp, &head, q)
> if (!strcmp(name, tp->s))
