[patch #3330] HURD_IHASH_ITERATE_KEYS

[Ognyan Kulev]

This mail is an automated notification from the patch tracker
 of the project: The GNU Hurd.

/**************************************************************************/
[patch #3330] Latest Modifications:

Changes by: 
                Ognyan Kulev <ogi@fmi.uni-sofia.bg>
'Date: 
                Wed 09/01/2004 at 13:28 (Europe/Sofia)

------------------ Additional Follow-up Comments ----------------------------
I don't need such macro for the ext2fs patch.  But I needed one for ext3fs.  
Fortunately, in my case, I could retrieve key from value.

Neal, I forgot about the break requirement.  I was too busy to think about the 
block requirement ;-)

Requiring KEY and VALUE to be defined by user seems OK to me: it looks 
"natural" and GCC will optimize when one of them is not used.






/**************************************************************************/
[patch #3330] Full Item Snapshot:

URL: <http://savannah.gnu.org/patch/?func=detailitem&item_id=3330>
Project: The GNU Hurd
Submitted by: Ognyan Kulev
On: Wed 09/01/2004 at 07:18

Category:  libihash
Priority:  3 - Low
Resolution:  None
Privacy:  Public
Assigned to:  marcus
Originator Email:  
Status:  Open


Summary:  HURD_IHASH_ITERATE_KEYS

Original Submission:  New for loop that loops only once is used.  Its purpose 
is to define key as hurd_ihash_key_t.

To loop once, another variable first_loop is used.  Even in -O1, disassembled 
source shows that GCC correctly optimizes the case and the variable is not 
defined at all.

Follow-up Comments
------------------


-------------------------------------------------------
Date: Wed 09/01/2004 at 13:28       By: Ognyan Kulev <ogi>
I don't need such macro for the ext2fs patch.  But I needed one for ext3fs.  
Fortunately, in my case, I could retrieve key from value.

Neal, I forgot about the break requirement.  I was too busy to think about the 
block requirement ;-)

Requiring KEY and VALUE to be defined by user seems OK to me: it looks 
"natural" and GCC will optimize when one of them is not used.

-------------------------------------------------------
Date: Wed 09/01/2004 at 13:17       By: Neal H. Walfield <neal>
Marcus, a single macro (which provides both the key and the value)
is also a good solution.  I don't think the
IHASH...(..., hurd_ihash_value_t val) form is terrible useful, however,
as we already require c99 and in which case you can just do:

    ...
    hurd_ihash_value_t val;
    IHASH...(..., val);

(i.e. you don't even need an enclosing block).

Thanks,
Neal


-------------------------------------------------------
Date: Wed 09/01/2004 at 12:19       By: Marcus Brinkmann <marcus>
Ogi, do you actually need this in your code?

In general, I don't object to make the key available, although if we do it, we 
could just as well do it always in a single macro, and we don't need two of 
them.

I'd still like to somehow retain the ability to do it in a local variable.  I 
am thinking about something like IHASH...(..., val) if you want the outside 
scope and IHASH...(..., hurd_ihash_value_t val) if you want a local variable, 
but I am not sure this can be achieved while at the same time retaining the 
block and break semantics.


-------------------------------------------------------
Date: Wed 09/01/2004 at 10:57       By: Neal H. Walfield <neal>
I agree this function is useful, however, I think the patch is
incorrect.  Marcus was very careful to make sure that a break in
HURD_IHASH_ITERATE would really break out of the iteration.  We should
strive to preserve this functionality in HURD_IHASH_ITERATE_KEYS.

The problem, of course, is that we can only declare a variables of a
single type in a for loop initializer section and Marcus automatically
declares the VAL variable for the user.  I think this is actually
detrimental as then it becomes inaccessible outside of the loop and
makes things like this impossible:

        hurd_ihash_t ht;
        HURD_IHASH_ITERATE (ht, val)
          if (condition1_p (val) || condition2_p (val))
            break;

        ...

        /* Examine val.  */

So, I think we should have the user declare VAL (and in the case of
your extension, KEY).

I have attached a patch which does this.  It updates libihash
but not the rest of the users.  If Marcus agrees then I can make a
patch to do this.

Here is a test case:

#include <stdio.h>
#include <ihash.h>

int
main (int argc, char *argv[])
{
  hurd_ihash_t ht;

  hurd_ihash_create (&ht, HURD_IHASH_NO_LOCP);

  hurd_ihash_add (ht, (hurd_ihash_key_t) 2, (hurd_ihash_value_t) -2);
  hurd_ihash_add (ht, (hurd_ihash_key_t) 3, (hurd_ihash_value_t) -3);
  hurd_ihash_add (ht, (hurd_ihash_key_t) 4, (hurd_ihash_value_t) -4);

  hurd_ihash_key_t key;
  hurd_ihash_value_t val;

  HURD_IHASH_ITERATE(ht, val)
    printf ("%dn", (int) val);

  HURD_IHASH_ITERATE_KEY(ht, key, val)
    printf ("%d -> %dn", (int) key, (int) val);

  return 0;
}







File Attachments
-------------------

-------------------------------------------------------
Date: Wed 09/01/2004 at 10:57  Name: ihash.diff  Size: 4.67KB   By: neal
Refactor HURD_IHASH_ITERATE; use a single for loop
http://savannah.gnu.org/patch/download.php?item_id=3330&amp;item_file_id=3627

-------------------------------------------------------
Date: Wed 09/01/2004 at 07:18  Name: ihash.patch  Size: 900B   By: ogi

http://savannah.gnu.org/patch/download.php?item_id=3330&amp;item_file_id=3625






For detailed info, follow this link:
<http://savannah.gnu.org/patch/?func=detailitem&item_id=3330>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/