[PATCH] maint.mk: prohibit inclusion of "hash.h" without_use
I spotted three more unnecessary #include directives in coreutils'
remove.c, so here's as syntax-check rule that would have detected one
of them:
>From e1b83155bc5e95f4165b806850e4117426479a68 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sun, 14 Feb 2010 11:22:29 +0100
Subject: [PATCH] maint.mk: prohibit inclusion of "hash.h" without_use
* top/maint.mk (sc_prohibit_hash_without_use): New rule.
---
ChangeLog|5 +
top/maint.mk | 11 +++
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c382d58..0ebee71 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2010-02-14 Jim Meyering
+
+ maint.mk: prohibit inclusion of "hash.h" without_use
+ * top/maint.mk (sc_prohibit_hash_without_use): New rule.
+
2010-02-10 Jim Meyering
maint.mk: prohibit inclusion of "ignore-value.h" without_use
diff --git a/top/maint.mk b/top/maint.mk
index ed8d367..a623d02 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -367,6 +367,17 @@ sc_prohibit_xalloc_without_use:
re='\<($(_xa1)|$(_xa2)) *\('\
$(_header_without_use)
+# Extract function names:
+# perl -lne '/^(?:extern )?(?:void|char) \*?(\w+) \(/ and print $1' lib/hash.h
+_hash_re = \
+clear|delete|free|get_(first|next)|insert|lookup|print_statistics|reset_tuning
+_hash_fn = \<($(_hash_re)) *\(
+_hash_struct = (struct )?\<[Hh]ash_(table|tuning)\>
+sc_prohibit_hash_without_use:
+ h='"hash.h"' \
+ re='$(_hash_fn)|$(_hash_struct)'\
+ $(_header_without_use)
+
sc_prohibit_safe_read_without_use:
@h='"safe-read.h"' re='(\|\
Re: [PATCH] Add hash_contains function
Colin Watson wrote:
> I looked at converting man-db to use Gnulib's hash implementation rather
> than its own. One obstacle seems to be that there is one place where
> man-db cares about the difference between a key not being in the hash at
> all, and a key being in the hash but with a NULL value (in Perl syntax,
> the difference between 'not exists $hash{$key}' and 'not defined
> $hash{$key}'). I haven't spent much time looking at the relevant code
> and so it may well be that it can be rewritten to avoid relying on this
> distinction, but in general it does seem like a useful distinction to
> draw; most high-level languages' built-in hash implementations allow you
> to ask whether a key exists without testing the truth of its value.
Hi Colin,
It's good to hear that man-db might use gnulib's hash function.
If the hash module were really unable to distinguish those cases, we
would have found it too limiting (and fixed it) long ago ;-)
The API for hash_insert requires that you pass it a non-NULL-pointer
(usually to a key/value pair). If you want to map a particular
"key" to NULL, that's fine: you'd call hash_insert with the address
of a {SOME_KEY,NULL} buffer (you define the lay-out). To look up that
key, you'd call hash_lookup with the pointer to a struct containing
{SOME_KEY,no-need-to-define}, and it will find the matching entry in
the table, returning a pointer, P, to the {SOME_KEY,NULL} buffer that
is in the table. You would then examine P->value to see whether
SOME_KEY's value is NULL.
Note that if you want to use the hash table as a simple set, there is
no need to have a separate key,value struct, since the value would be
irrelevant. In that case, you can save storage and indirection costs
by passing the key string directly to hash_insert (always assuming the
hash_compare and hash_hash functions are aware).
The bottom line is that hash_lookup cannot return NULL for a good
reason. One cannot call hash_insert with a NULL pointer, so hash_lookup
could never return NULL for a matching entry in the hash table.
Perhaps this can be attributed to the lack of documentation and/or
examples? For reference, there are many uses in gnulib itself
and in the tar and coreutils packages.
Jim
emit_upload_commands target doesn't work for savannah.nongnu.org
Hi, I'm using gnulib for the autoconf-archive project. To upload a new release, I have to use the following invocation of gnupload: ./build-aux/gnupload --to dl.sv.nongnu.org:/releases/autoconf-archive/ file1 file2 ... Now, it doesn't seem possible to configure gnulib's emit_upload_commands target to produce that command correctly. I've tried setting $(gnu_rel_host) to "dl.sv.nongnu.org:/releases/autoconf-archive/", but unfortunately emit_upload_commands appends the contents of $(PACKAGE) to that variable unconditionally: emit_upload_commands: @echo = @echo = @echo "$(build_aux)/gnupload $(GNUPLOADFLAGS) \\" @echo "--to $(gnu_rel_host):$(PACKAGE) \\" @echo " $(rel-files)" @echo '# send the ~/announce-$(my_distdir) e-mail' @echo = @echo = In case of the Autoconf Archive, the outcome of "$(gnu_rel_host):$(PACKAGE)" is never going to be correct, because there of the '/releases' bit at the beginning of the path. Does anyone have a suggestion how to best remedy that problem? Take care, Peter
Re: emit_upload_commands target doesn't work for savannah.nongnu.org
Peter Simons wrote: > I'm using gnulib for the autoconf-archive project. To upload a new release, > I have to use the following invocation of gnupload: > > ./build-aux/gnupload --to dl.sv.nongnu.org:/releases/autoconf-archive/ > file1 file2 ... > > Now, it doesn't seem possible to configure gnulib's emit_upload_commands > target to produce that command correctly. I've tried setting $(gnu_rel_host) > to "dl.sv.nongnu.org:/releases/autoconf-archive/", but unfortunately > emit_upload_commands appends the contents of $(PACKAGE) to that variable > unconditionally: > > emit_upload_commands: > @echo = > @echo = > @echo "$(build_aux)/gnupload $(GNUPLOADFLAGS) \\" > @echo "--to $(gnu_rel_host):$(PACKAGE) \\" > @echo " $(rel-files)" > @echo '# send the ~/announce-$(my_distdir) e-mail' > @echo = > @echo = > > In case of the Autoconf Archive, the outcome of "$(gnu_rel_host):$(PACKAGE)" > is never going to be correct, because there of the '/releases' bit at the > beginning of the path. > > Does anyone have a suggestion how to best remedy that problem? Hi Peter, How about this? >From 01944b9984e7a297829374dfbb35aadbc559102a Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 14 Feb 2010 16:42:22 +0100 Subject: [PATCH] maint.mk: allow the default upload destination dir to be overridden * top/maint.mk (upload_dest_dir_): Define with a default that preserves the status quo. (emit_upload_commands): Use it, rather than hard-coding $(PACKAGE). Reported by Peter Simons. --- ChangeLog|6 ++ top/maint.mk |3 ++- 2 files changed, 8 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0ebee71..8fdabc2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2010-02-14 Jim Meyering + maint.mk: allow the default upload destination dir to be overridden + * top/maint.mk (upload_dest_dir_): Define with a default that + preserves the status quo. + (emit_upload_commands): Use it, rather than hard-coding $(PACKAGE). + Reported by Peter Simons. + maint.mk: prohibit inclusion of "hash.h" without_use * top/maint.mk (sc_prohibit_hash_without_use): New rule. diff --git a/top/maint.mk b/top/maint.mk index a623d02..cfe0fd7 100644 --- a/top/maint.mk +++ b/top/maint.mk @@ -798,11 +798,12 @@ announcement: NEWS ChangeLog $(rel-files) ftp-gnu = ftp://ftp.gnu.org/gnu www-gnu = http://www.gnu.org +upload_dest_dir_ ?= $(PACKAGE) emit_upload_commands: @echo = @echo = @echo "$(build_aux)/gnupload $(GNUPLOADFLAGS) \\" - @echo "--to $(gnu_rel_host):$(PACKAGE) \\" + @echo "--to $(gnu_rel_host):$(upload_dest_dir_) \\" @echo " $(rel-files)" @echo '# send the ~/announce-$(my_distdir) e-mail' @echo = -- 1.7.0.169.g57c99
Re: emit_upload_commands target doesn't work for savannah.nongnu.org
Hi Jim, yes, that change would solve my problem. Thank you very much. Take care, Peter
Re: emit_upload_commands target doesn't work for savannah.nongnu.org
Peter Simons wrote: > yes, that change would solve my problem. Thank you very much. Ok. Pushed.
Re: [PATCH] Add hash_contains function
On Sun, Feb 14, 2010 at 03:02:14PM +0100, Jim Meyering wrote:
> Colin Watson wrote:
> > I looked at converting man-db to use Gnulib's hash implementation rather
> > than its own. One obstacle seems to be that there is one place where
> > man-db cares about the difference between a key not being in the hash at
> > all, and a key being in the hash but with a NULL value (in Perl syntax,
> > the difference between 'not exists $hash{$key}' and 'not defined
> > $hash{$key}'). I haven't spent much time looking at the relevant code
> > and so it may well be that it can be rewritten to avoid relying on this
> > distinction, but in general it does seem like a useful distinction to
> > draw; most high-level languages' built-in hash implementations allow you
> > to ask whether a key exists without testing the truth of its value.
>
> Hi Colin,
>
> It's good to hear that man-db might use gnulib's hash function.
>
> If the hash module were really unable to distinguish those cases, we
> would have found it too limiting (and fixed it) long ago ;-)
I confess I did not check how new the module was; written in 1992, I can
see your point. ;-)
> The API for hash_insert requires that you pass it a non-NULL-pointer
> (usually to a key/value pair).
Oops. My bad for only code-inspecting the lookup functions ... the
hash_insert documentation is of course perfectly clear.
> Note that if you want to use the hash table as a simple set, there is
> no need to have a separate key,value struct, since the value would be
> irrelevant. In that case, you can save storage and indirection costs
> by passing the key string directly to hash_insert (always assuming the
> hash_compare and hash_hash functions are aware).
Indeed, in the case at hand it looks as though I just need a set, so
you're right, setting key == value will be quite adequate.
> Perhaps this can be attributed to the lack of documentation and/or
> examples? For reference, there are many uses in gnulib itself
> and in the tar and coreutils packages.
I think the biggest thing I found missing in this regard was an overview
of the generic data structure modules provided by Gnulib. For instance,
I wasted some time fiddling around with oset before realising that I
didn't really need an *ordered* set in most cases and that a hash would
do just fine. Some kind of index of this category of modules would be
wonderful, for orientation purposes.
Thanks for the cluebat,
--
Colin Watson [cjwat...@debian.org]
