hash.h: HASH_DO_REMOVE and HASH_INSERT macros can not be called from HASH_WALK, assert it with calling bug().

lib/hash.h

@@ -10,7 +10,7 @@
 #ifndef _BIRD_HASH_H_
 #define _BIRD_HASH_H_
 
-#define HASH(type)		struct { type **data; uint count, order; }
+#define HASH(type)		struct { type **data; uint count, order; char* is_in_walk; }
 #define HASH_TYPE(v)		typeof(** (v).data)
 #define HASH_SIZE(v)		(1U << (v).order)
 
@@ -23,6 +23,8 @@
     (v).count = 0;							\
     (v).order = (init_order);						\
     (v).data = mb_allocz(pool, HASH_SIZE(v) * sizeof(* (v).data));	\
+    (v).is_in_walk = mb_allocz(pool, sizeof(char));			\
+    *(v).is_in_walk = 0;						\
   })
 
 #define HASH_FREE(v)							\
@@ -42,6 +44,8 @@
 
 #define HASH_INSERT(v,id,node)						\
   ({									\
+    if (*(v).is_in_walk)						\
+      bug("HASH_INSERT: Attempt to insert in HASH_WALK");		\
     u32 _h = HASH_FN(v, id, id##_KEY((node)));				\
     HASH_TYPE(v) **_nn = (v).data + _h;					\
     id##_NEXT(node) = *_nn;						\
@@ -51,6 +55,8 @@
 
 #define HASH_DO_REMOVE(v,id,_nn)					\
   ({									\
+    if (*(v).is_in_walk)						\
+      bug("HASH_DELETE: Attempt to remove in HASH_WALK");		\
     *_nn = id##_NEXT((*_nn));						\
     (v).count--;							\
   })
@@ -165,12 +171,13 @@
 #define HASH_WALK(v,next,n)						\
   do {									\
     HASH_TYPE(v) *n;							\
+    *(v).is_in_walk = 1;						\
     uint _i;								\
     uint _s = HASH_SIZE(v);						\
     for (_i = 0; _i < _s; _i++)						\
       for (n = (v).data[_i]; n; n = n->next)
 
-#define HASH_WALK_END } while (0)
+#define HASH_WALK_END(v) *(v).is_in_walk = 0; } while (0)
 
 
 #define HASH_WALK_DELSAFE(v,next,n)					\

lib/hash_test.c

@@ -185,7 +185,7 @@ t_walk(void)
   {
     check[n->key]++;
   }
-  HASH_WALK_END;
+  HASH_WALK_END(hash);
 
   for (i = 0; i < MAX_NUM; i++)
     bt_assert(check[i] == 1);
@@ -285,6 +285,26 @@ t_walk_filter(void)
   return 1;
 }
 
+void
+do_walk_delete_error(void)
+{
+    init_hash();
+  fill_hash();
+
+  HASH_WALK(hash, next, n)
+  {
+    HASH_DELETE(hash, TEST, n->key);
+  }
+  HASH_WALK_END(hash);
+}
+
+static int
+t_walk_check_bug(void)
+{
+  return bt_assert_bug(do_walk_delete_error, "HASH_DELETE: Attempt to remove in HASH_WALK");
+}
+
+
 int
 main(int argc, char *argv[])
 {
@@ -299,6 +319,7 @@ main(int argc, char *argv[])
   bt_test_suite(t_walk_delsafe_remove, 	"HASH_WALK_DELSAFE and HASH_REMOVE");
   bt_test_suite(t_walk_delsafe_remove2,	"HASH_WALK_DELSAFE and HASH_REMOVE2. HASH_REMOVE2 is HASH_REMOVE and smart auto-resize function");
   bt_test_suite(t_walk_filter,		"HASH_WALK_FILTER");
+  bt_test_suite(t_walk_check_bug,	"HASH_DO_REMOVE returns error, because called from HASH_WALK");
 
   return bt_exit_value();
 }

nest/cmds.c

@@ -62,7 +62,7 @@ cmd_show_symbols(struct sym_show_data *sd)
 
 	cli_msg(-1010, "%-8s\t%s", sym->name, cf_symbol_class_name(sym));
       }
-      HASH_WALK_END;
+      HASH_WALK_END(scope->hash);
 
     cli_msg(0, "");
   }

nest/mpls.c

@@ -755,7 +755,7 @@ mpls_fec_map_reconfigure(struct mpls_fec_map *m, struct channel *C)
     mpls_unlink_fec(m, fec);
     fec->policy = MPLS_POLICY_NONE;
   }
-  HASH_WALK_END;
+  HASH_WALK_END(m->label_hash);
 
   /* Remove old unused handles */
 
@@ -777,7 +777,7 @@ mpls_fec_map_free(struct mpls_fec_map *m)
       rta_free(fec->rta);
       fec->rta = NULL;
     }
-    HASH_WALK_END;
+    HASH_WALK_END(m->rta_hash);
   }
 
   /* Free allocated labels */
@@ -788,7 +788,7 @@ mpls_fec_map_free(struct mpls_fec_map *m)
     if (!fec->policy && !fec->handle->label_count)
       mpls_free_handle(m->domain, fec->handle);
   }
-  HASH_WALK_END;
+  HASH_WALK_END(m->label_hash);
 
   if (m->static_handle)
     mpls_free_handle(m->domain, m->static_handle);

proto/aggregator/aggregator.c

@@ -254,7 +254,7 @@ aggregator_reload_buckets(void *data)
       aggregator_bucket_update(p, b, b->rte->net);
       lp_flush(rte_update_pool);
     }
-  HASH_WALK_END;
+  HASH_WALK_END(p->buckets);
 }
 
 
@@ -732,7 +732,7 @@ aggregator_shutdown(struct proto *P)
     HASH_REMOVE(p->buckets, AGGR_BUCK, b);
     sl_free(b);
   }
-  HASH_WALK_END;
+  HASH_WALK_END(p->buckets);
 
   return PS_DOWN;
 }

proto/bfd/bfd.c

@@ -764,7 +764,7 @@ bfd_drop_requests(struct bfd_proto *p)
     WALK_LIST_FIRST(n, s->request_list)
       bfd_submit_request(SKIP_BACK(struct bfd_request, n, n));
   }
-  HASH_WALK_END;
+  HASH_WALK_END(p->session_hash_id);
 }
 
 static struct resclass bfd_request_class;
@@ -1162,7 +1162,7 @@ bfd_reconfigure(struct proto *P, struct proto_config *c)
     if (s->ifa->changed)
       bfd_reconfigure_session(p, s);
   }
-  HASH_WALK_END;
+  HASH_WALK_END(p->session_hash_id);
 
   bfd_reconfigure_neighbors(p, new);
 
@@ -1289,7 +1289,7 @@ bfd_show_sessions(struct proto *P, struct bfd_show_sessions_cmd *args)
 
     bfd_show_session(s, args->verbose);
   }
-  HASH_WALK_END;
+  HASH_WALK_END(p->session_hash_id);
 }