Re: [PATCH] argv-iter: new module

[Jim Meyering]

Bruno Haible wrote:
> Hi Jim,
>
> For a module that is to be used by several package, some more documentation
> is needed IMO. From the comments outside of functions that are present, I can
> only get a vague impression that the module is about "Iterate over arguments
> from argv or --files0-from=FILE" and that file has is supposed to contain
> NUL-delimited items.
>
> Specifically, the questions that prevent from using the API are:
>
> - What does the type 'struct argv_iterator' stand for?
> - argv_iter_err appears to be some error code type. What do AI_ERR_MEM and
>   AI_ERR_READ mean?
> - What does argv_iter_init_argv do?
> - Can the contents of 'argv' be freed while the result of argv_iter_init_argv
>   is still in use?
> - About argv_iter_init_stream? "Initialize to read from the stream" What
>   is being initialized? The return value or some undocumented hidden state?
> - Still about argv_iter_init_stream: "The input is expected to contain a
>   list of NUL-delimited tokens". What happens if the input starts with a
>   NULL? Or when it ends with a non-NULL byte? In other words, is it really
>   "NUL-delimited tokens" or "NUL-terminated tokens"?
> - Still about argv_iter_init_stream: Does this function close the stream,
>   or is it the caller's duty?
> - What does argv_iter do?
> - Is the argv_iter result freshly allocated, i.e. must the caller free it?
> - What does argv_iter_n_args do?
> - What does argv_iter_free do? Does it free the original 'char **argv'
>   argument and/or close the stream?

Hi Bruno,

Thanks for the pointed questions.
Yes, I will add some documentation.

> About the unit test:
>
>> +#define STREQ(s1, s2) ((strcmp (s1, s2) == 0))
>
> What is the reason for the extra parentheses? Is some gcc warning avoided
> by this?

I must have copied that definition from some other
code that had the same problem.

As far as I know, there is never a need for more than one
set of parentheses around an "expr1 == expr2" expression.
So I'll correct them with this:

>From 1203e8d1f62dec3d2436dffadd5c20793cf84366 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 5 Jul 2009 17:45:56 +0200
Subject: [PATCH] remove superfluous parentheses in STREQ definition

* tests/test-argv-iter.c (STREQ): Remove redundant parentheses.
* lib/getugroups.c (STREQ): Likewise.
* lib/fnmatch.c (STREQ): Likewise.
Spotted by Bruno Haible.
---
 ChangeLog              |    8 ++++++++
 lib/fnmatch.c          |    4 ++--
 lib/getugroups.c       |    4 ++--
 tests/test-argv-iter.c |    2 +-
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1dc20c0..1ada00b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-07-05  Jim Meyering  <address@hidden>
+
+       remove superfluous parentheses in STREQ definition
+       * tests/test-argv-iter.c (STREQ): Remove redundant parentheses.
+       * lib/getugroups.c (STREQ): Likewise.
+       * lib/fnmatch.c (STREQ): Likewise.
+       Spotted by Bruno Haible.
+
 2009-07-04  Jim Meyering  <address@hidden>

        argv-iter: new module
diff --git a/lib/fnmatch.c b/lib/fnmatch.c
index 48bc8b5..a0356ac 100644
--- a/lib/fnmatch.c
+++ b/lib/fnmatch.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 
1991,1992,1993,1996,1997,1998,1999,2000,2001,2002,2003,2004,2005,2006,2007
+/* Copyright (C) 
1991,1992,1993,1996,1997,1998,1999,2000,2001,2002,2003,2004,2005,2006,2007,2009
        Free Software Foundation, Inc.

    This program is free software; you can redistribute it and/or modify
@@ -89,7 +89,7 @@ extern int fnmatch (const char *pattern, const char *string, 
int flags);
 #  define isblank(c) ((c) == ' ' || (c) == '\t')
 # endif

-# define STREQ(s1, s2) ((strcmp (s1, s2) == 0))
+# define STREQ(s1, s2) (strcmp (s1, s2) == 0)

 # if defined _LIBC || WIDE_CHAR_SUPPORT
 /* The GNU C library provides support for user-defined character classes
diff --git a/lib/getugroups.c b/lib/getugroups.c
index b266bb8..2207b85 100644
--- a/lib/getugroups.c
+++ b/lib/getugroups.c
@@ -1,6 +1,6 @@
 /* getugroups.c -- return a list of the groups a user is in

-   Copyright (C) 1990, 1991, 1998-2000, 2003-2008 Free Software Foundation.
+   Copyright (C) 1990, 1991, 1998-2000, 2003-2009 Free Software Foundation.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -36,7 +36,7 @@ struct group *getgrent (void);

 #include <string.h>

-#define STREQ(s1, s2) ((strcmp (s1, s2) == 0))
+#define STREQ(s1, s2) (strcmp (s1, s2) == 0)

 /* Like `getgroups', but for user USERNAME instead of for the current
    process.  Store at most MAXCOUNT group IDs in the GROUPLIST array.
diff --git a/tests/test-argv-iter.c b/tests/test-argv-iter.c
index 7682c4a..5070754 100644
--- a/tests/test-argv-iter.c
+++ b/tests/test-argv-iter.c
@@ -22,7 +22,7 @@
 #include <string.h>

 #define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array))
-#define STREQ(s1, s2) ((strcmp (s1, s2) == 0))
+#define STREQ(s1, s2) (strcmp (s1, s2) == 0)
 #define ASSERT(expr) \
   do                                                                         \
     {                                                                        \
--
1.6.3.3.507.gc6b5a