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 <[email protected]> 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ü>
