The change applied is not enough. It might pass through the test though, =
because the fill-bucket and remove-from-bucket doesn't use key2.
If key1 differs in dkhash_remove, the continue-statment is triggered, =
which skips the prev=3Dbkt-statement.
I haven't synced the code with svn yet, but just updating with the =
changes applied in commit 2612, the test actually failes. (same as =
before)
The only time this patch helps is if you add all values with equal k1, =
but k2 differs, then the second if statement in the loop exits and =
prev=3Dbkt runs.
--=20
Max Sikstr=F6m
developer - op5 AB
[email protected]
http://www.op5.com/
On Feb 20, 2013, at 7:03 PM, Andreas Ericsson wrote:
> Nice catch. I'll remove the unrelated changes and apply.
>=20
> Thanks.
>=20
> On 02/20/2013 05:48 PM, [email protected] wrote:
>> From: Max Sikstr=F6m
>>=20
>> Pointer to previous in dkhash_remove was only set for first element =
in a given
>> bucket. So when finding for example third object for removal, setting
>> prev->next=3Dobj->next looses the reference to the old prev->next, =
which is the
>> second object in the list.
>>=20
>> This can be tested by creating a hash map of one bucket (practically =
a linked
>> list), and fill and remove all, both forward and backward.
>>=20
>> Signed-off-by: Max Sikstr=F6m
>> ---
>> lib/dkhash.c | 5 ++---
>> lib/test-dkhash.c | 36 +++++++++++++++++++++++++++++++++++-
>> 2 files changed, 37 insertions(+), 4 deletions(-)
>>=20
>> diff --git a/lib/dkhash.c b/lib/dkhash.c
>> index cab3d66..74a9a76 100644
>> --- a/lib/dkhash.c
>> +++ b/lib/dkhash.c
>> @@ -209,9 +209,7 @@ void *dkhash_remove(dkhash_table *t, const char =
*k1, const char *k2)
>>=20
>> prev =3D bkt; /* pay attention */
>> for (; bkt; bkt =3D bkt->next) {
>> - if (strcmp(k1, bkt->key))
>> - continue;
>> - if ((!k2 && !bkt->key2) || !strcmp(k2, bkt->key2)) {
>> + if (!strcmp(k1, bkt->key) && ((!k2 && !bkt->key2) || =
!strcmp(k2, bkt->key2)) ) {
>> if (prev =3D=3D bkt) {
>> /* first entry deleted */
>> t->buckets[slot] =3D bkt->next;
>> @@ -223,6 +221,7 @@ void *dkhash_remove(dkhash_table *t, const char =
*k1, const char *k2)
>> t->removed++;
>> return dkhash_destroy_bucket(bkt);
>> }
>> + prev =3D bkt;
>> }
>>=20
>> return NULL;
>> diff --git a/lib/test-dkhash.c b/lib/test-dkhash.c
>> index 02accba..313b361 100644
>> --- a/lib/test-dkhash.c
>> +++ b/lib/test-dkhash.c
>> @@ -87,10 +87,13 @@ static int del_matching(void *data)
>> int main(int argc, char **argv)
>> {
>> dkhash_table *tx, *t;
>> - unsigned int x;
>> + int x;
>> struct test_data s;
>> char *p1, *p2;
>>=20
>> + char *strs[10];
>> + char tmp[32];
>> +
>> t_set_colors(0);
>> t_start("dkhash basic test");
>> t =3D dkhash_create(512);
>> @@ -143,6 +146,37 @@ int main(int argc, char **argv)
>> test(0 =3D=3D dkhash_num_entries(tx), "x table post all ops");
>> test(0 =3D=3D dkhash_check_table(tx), "x table consistency post =
all ops");
>> dkhash_debug_table(tx, 0);
>> + t_end();
>> +
>> + for(x=3D0;x> + sprintf(tmp, "string %d", x);
>> + strs[x] =3D strdup(tmp);
>> + }
>> +
>> + t_start("dkhash single bucket add remove forward");
>> +
>> + t =3D dkhash_create(1);
>> + for(x=3D0;x> + dkhash_insert( t, strs[x], NULL, strs[x] );
>> + }
>> + for(x=3D0;x> + p1 =3D strs[x];
>> + p2 =3D dkhash_remove( t, p1, NULL );
>> + test( p1 =3D=3D p2, "remove should return a value" );
>> + }
>> + t_end();
>> +
>> + t_start("dkhash single bucket add remove backward");
>> +
>> + t =3D dkhash_create(1);
>> + for(x=3D0;x> + dkhash_insert( t, strs[x], NULL, strs[x] );
>> + }
>> + for(x=3D9;x>=3D0;x--) {
>> + p1 =3D strs[x];
>> + p2 =3D dkhash_remove( t, p1, NULL );
>> + test( p1 =3D=3D p2, "remove should return a value" );
>> + }
>>=20
>> return t_end();
>> }
>>=20
>=20
>=20
> --=20
> Andreas Ericsson [email protected]
> OP5 AB www.op5.se
> Tel: +46 8-230225
...[email truncated]...
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]