[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[patch #3330] HURD_IHASH_ITERATE_KEYS
From: |
Neal H. Walfield |
Subject: |
[patch #3330] HURD_IHASH_ITERATE_KEYS |
Date: |
Wed, 01 Sep 2004 07:02:26 -0400 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5 |
This mail is an automated notification from the patch tracker
of the project: The GNU Hurd.
/**************************************************************************/
[patch #3330] Latest Modifications:
Changes by:
Neal H. Walfield <neal@cs.uml.edu>
'Date:
Wed 09/01/04 at 10:57 (GMT)
What | Removed | Added
---------------------------------------------------------------------------
Assigned to | None | marcus
------------------ Additional Follow-up Comments ----------------------------
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;
}
/**************************************************************************/
[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/04 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/04 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/04 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&item_file_id=3627
-------------------------------------------------------
Date: Wed 09/01/04 at 07:18 Name: ihash.patch Size: 900B By: ogi
http://savannah.gnu.org/patch/download.php?item_id=3330&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/