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

[Jim Meyering]

Bruno Haible wrote:

> 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.

As Paul said, I think we can be confident that this
is not a problem in practice.

> 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.

I don't know why di_set_insert uses "(void *)".
Obviously an oversight, since it should be const.
Apparently not all gcc warnings are enabled.
I've fixed it.  Pushed patch below.

> 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.

Fine as well.  Thanks!

>From 489cedffb410a94803cf10502b27b1facf026dfc Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 8 Feb 2011 08:57:36 +0100
Subject: [PATCH] di-set: add "const" to a cast

* lib/di-set.c (di_set_insert): Cast hash_insert0 argument to
"(void const *)", not "(void *)".  Spotted by Bruno Haible.
---
 ChangeLog    |    6 ++++++
 lib/di-set.c |    2 +-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c1ba821..2703b2d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-02-08  Jim Meyering  <address@hidden>
+
+       di-set: add "const" to a cast
+       * lib/di-set.c (di_set_insert): Cast hash_insert0 argument to
+       "(void const *)", not "(void *)".  Spotted by Bruno Haible.
+
 2011-02-06  Bruno Haible  <address@hidden>

        Rename module 'wctype' to 'wctype-h'.
diff --git a/lib/di-set.c b/lib/di-set.c
index 5e839a3..4730e75 100644
--- a/lib/di-set.c
+++ b/lib/di-set.c
@@ -233,7 +233,7 @@ di_set_insert (struct di_set *dis, dev_t dev, ino_t ino)
     return -1;

   /* Put I into the inode set.  */
-  return hash_insert0 (ino_set, (void *) i, NULL);
+  return hash_insert0 (ino_set, (void const *) i, NULL);
 }

 /* Look up the DEV,INO pair in the set DIS.
--
1.7.4.2.g597a6