Re: ihash_add

[Marcus Brinkmann]

On Mon, Aug 20, 2001 at 02:04:22PM -0700, Thomas Bushnell, BSG wrote:
> > So I think, that's a bug.  If you agree, I would like to fix it and rewrite
> > the loop a bit (initialize h and firsth to HASH(ht, id), and replace
> > the for with a simple while loop).  Also using index_empty.
> > I think it is unnecessary convoluted right now.
> 
> But I have no objection to adding a break.
> 
> Why do you think writing this as a while loop would make it less
> convoluted?  It would stay exactly the same AFAICT.

My main point is the lazy initialization of firsth, which seems to be
unnecessary.  As I tend to read code from beginning to end, it was 
also something that confused me ;)
The while I suggested because if you initialize h and firsth before the
loop, you don't need the initializer and you also loose the block, and
with only a break condition you can move the REHASH into the block of a
while nicely.  It doesn't make a huge difference.

I should provide a patch to make it clearer, but it's not convenient for
me to do that right now.  While we are talking about ihash, I think it would
be better to change the interface of find_index to return -1 if it was not
found and the index otherwise.  This way you don't need to check if it is
valid.  What do you think?

In general, my plan was to read the whole of ihash.c and ponder it, in case
there are any more bugs.  In fact, we know from the proc bug that the hash
grows faster then it should: Although the number of processes running at the
same time remains constant, the hash table grows (otherwise, the broken
seive code wouldn't have been called).  But I don't have much info about it
right now.  Part might be due to dead processes that are not reaped.

Thanks,
Marcus