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

[Bruno Haible]

Jim Meyering wrote:
> * lib/di-set.c: New file.
> * lib/di-set.h: Likewise.
> * modules/di-set: Likewise.
> * modules/di-set-tests: Likewise.
> * tests/test-di-set.c: Likewise.

A copyright header would make sense in lib/di-set.h.

In lib/di-set.c, function di_ent_hash, there is the assumption that
dev_t is an integral type. But POSIX
<http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html>
does not guarantee this. It could also be a floating-point type.

Also, comparing lines 236 and 258:
  return hash_insert0 (ino_set, (void *) i, NULL);
  return !!hash_lookup (ino_set, (void const *) i);
Why is a 'void const *' used in the second case, but not in the first case?
hash_insert0 takes a 'void const *' parameter as well.

A proposed patch for the tests, similar to the one for ino-map:


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

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

--- modules/di-set-tests.orig   Tue Feb  8 01:43:16 2011
+++ modules/di-set-tests        Tue Feb  8 01:24:57 2011
@@ -1,5 +1,6 @@
 Files:
 tests/test-di-set.c
+tests/macros.h
 
 Depends-on:
 
--- tests/test-di-set.c.orig    Tue Feb  8 01:43:16 2011
+++ tests/test-di-set.c Tue Feb  8 01:25:55 2011
@@ -17,25 +17,11 @@
 /* Written by Jim Meyering.  */
 
 #include <config.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
-#include <stdint.h>
-
-#define ASSERT(expr) \
-  do                                                                         \
-    {                                                                        \
-      if (!(expr))                                                           \
-        {                                                                    \
-          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
-          fflush (stderr);                                                   \
-          abort ();                                                          \
-        }                                                                    \
-    }                                                                        \
-  while (0)
 
 #include "di-set.h"
 
+#include "macros.h"
+
 int
 main (void)
 {
@@ -53,11 +39,13 @@
   ASSERT (di_set_insert (dis, 5, (ino_t) -1) == 1);
   ASSERT (di_set_insert (dis, 5, (ino_t) -1) == 0); /* dup */
 
-  unsigned int i;
-  for (i = 0; i < 3000; i++)
-    ASSERT (di_set_insert (dis, 9, i) == 1);
-  for (i = 0; i < 3000; i++)
-    ASSERT (di_set_insert (dis, 9, i) == 0); /* duplicate fails */
+  {
+    unsigned int i;
+    for (i = 0; i < 3000; i++)
+      ASSERT (di_set_insert (dis, 9, i) == 1);
+    for (i = 0; i < 3000; i++)
+      ASSERT (di_set_insert (dis, 9, i) == 0); /* duplicate fails */
+  }
 
   di_set_free (dis);
 

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