Re: [PATCH] di-set, ino-map: new modules, from coreutils

[Bruno Haible]

Jim Meyering wrote:
> * lib/ino-map.c: Likewise.
> * lib/ino-map.h: Likewise.
> * modules/ino-map: Likewise.
> * modules/ino-map-tests: Likewise.
> * tests/test-ino-map.c: Likewise.

lib/ino-map.h could well use a copyright header.

lib/ino-map.c looks 100% correct.

For the tests, here's a suggested patch that addresses three issues:

  - If ino-map.h were included right after config.h, it would be a test of its
    self-containedness.

  - ASSERT is now found in macros.h. The comment
    /* FIXME: once/if in gnulib, use #include "macros.h" in place of this */
    looks like you thought that this change was only necessary after moving
    the module to gnulib. Not so: You can have a module that contains two
    files
      tests/test-ino-map.c
      tests/macros.h
    where the first file comes from coreutils and the second file comes from
    gnulib. Absolutely no problem.

  - Variable declaration after statement: a C99 feature.


2011-02-07  Bruno Haible  <address@hidden>

        ino-map tests: refactor
        * tests/test-ino-map.c: Include ino-map.h early. Include macros.h. Drop
        unnecessary includes.
        (ASSERT): Remove macro.
        (main): Make C90 compliant by avoiding variable declaration after
        statement.
        * modules/ino-map-tests (Files): Add tests/macros.h.

--- modules/ino-map-tests.orig  Tue Feb  8 01:16:09 2011
+++ modules/ino-map-tests       Tue Feb  8 01:13:06 2011
@@ -1,5 +1,6 @@
 Files:
 tests/test-ino-map.c
+tests/macros.h
 
 Depends-on:
 
--- tests/test-ino-map.c.orig   Tue Feb  8 01:16:09 2011
+++ tests/test-ino-map.c        Tue Feb  8 01:13:44 2011
@@ -17,25 +17,11 @@
 /* Written by Jim Meyering.  */
 
 #include <config.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-
-/* FIXME: once/if in gnulib, use #include "macros.h" in place of this */
-#define ASSERT(expr) \
-  do                                                                         \
-    {                                                                        \
-      if (!(expr))                                                           \
-        {                                                                    \
-          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
-          fflush (stderr);                                                   \
-          abort ();                                                          \
-        }                                                                    \
-    }                                                                        \
-  while (0)
 
 #include "ino-map.h"
 
+#include "macros.h"
+
 int
 main ()
 {
@@ -50,11 +36,13 @@
   ASSERT (ino_map_insert (ino_map, 0) == INO_MAP_INIT + 2);
   ASSERT (ino_map_insert (ino_map, 0) == INO_MAP_INIT + 2);
 
-  int i;
-  for (i = 0; i < 100; i++)
-    {
-      ASSERT (ino_map_insert (ino_map, 10000 + i) == INO_MAP_INIT + 3 + i);
-    }
+  {
+    int i;
+    for (i = 0; i < 100; i++)
+      {
+        ASSERT (ino_map_insert (ino_map, 10000 + i) == INO_MAP_INIT + 3 + i);
+      }
+  }
 
   ino_map_free (ino_map);
 

-- 
In memoriam Hatun Sürücü <http://en.wikipedia.org/wiki/Hatun_Sürücü>