[Cocci] surprising SmPL result

[Pierre Habouzit]

We have a linux-like iterator named qh_for_each_pos that is an iterator
over hash tables. Hash tables have two operations: qh_del_key and
qh_del_pos. The former removes an element by key, the second one by
position in the underlying low level array that is used for our
double-hashing implementation.

Though when the key is numeric it's easy to use one for the other which
is wrong. I wrote the following SmPL:

@@
iterator name qh_for_each_pos;
expression name, it, qh, e;
@@
qh_for_each_pos(name, it, qh) {
<+...
(
-qh_del_key(name, qh, it)
+qh_del_at(name, qh, it)
|
qh_del_at(name, qh, it)
|
-qh_del_at(name, qh, e)
+qh_del_at_IS_BUGGY(name, qh, e)
)
...+>
}

And tested it with this C:

            qh_for_each_pos(status_line, lpos, &cp->status_line) {
                sb_addf(cur_buf, "%d;", cp->status_line.keys[lpos]);
                qh_del_key(status_line, &cp->status_line, lpos);
                qh_del_at(status_line, &cp->status_line, lpos);
                qh_del_at(status_line, &cp->status_line, 10);
            }

Surprisingly the result patch was:
--- mcms/backend.c	2009-11-26 09:55:50.273396942 +0100
+++ /tmp/cocci-output-16559-166c68-backend.c	2009-11-26 10:05:38.477901644 +0100
@@ -931,9 +931,9 @@ int be_campaign_cache_save(be_campaign_c
 
             qh_for_each_pos(status_line, lpos, &cp->status_line) {
                 sb_addf(cur_buf, "%d;", cp->status_line.keys[lpos]);
-                qh_del_key(status_line, &cp->status_line, lpos);
                 qh_del_at(status_line, &cp->status_line, lpos);
-                qh_del_at(status_line, &cp->status_line, 10);
+                qh_del_at(status_line, &cp->status_line, lpos);
+                qh_del_at_IS_BUGGY(status_line, &cp->status_line, 10);
             }


Where I would have expected:
--- mcms/backend.c	2009-11-26 09:55:50.273396942 +0100
+++ /tmp/cocci-output-16559-166c68-backend.c	2009-11-26 10:05:38.477901644 +0100
@@ -931,9 +931,9 @@ int be_campaign_cache_save(be_campaign_c
 
             qh_for_each_pos(status_line, lpos, &cp->status_line) {
                 sb_addf(cur_buf, "%d;", cp->status_line.keys[lpos]);
-                qh_del_key(status_line, &cp->status_line, lpos);
+                qh_del_at(status_line, &cp->status_line, lpos);
                 qh_del_at(status_line, &cp->status_line, lpos);
-                qh_del_at(status_line, &cp->status_line, 10);
+                qh_del_at_IS_BUGGY(status_line, &cp->status_line, 10);
             }

If you put a statement around the qh_del_at that shouldn't change, then I get
the intended result.



-- 
Intersec <http://www.intersec.com>
Pierre Habouzit <pierre.habouzit at intersec.com>
Tél : +33 (0)1 5570 3346
Mob : +33 (0)6 1636 8131
Fax : +33 (0)1 5570 3332
37 Rue Pierre Lhomme
92400 Courbevoie