Bruno Haible wrote: > 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.
It's so short and simple, adding a copyright header seems unwarranted. > 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. I've done that for the remaining definition of ASSERT in coreutils. > - Variable declaration after statement: a C99 feature. > > > 2011-02-07 Bruno Haible <[email protected]> > > 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. Thank you for the review and patch. Those changes look fine.
