[TIPC]: First phase of assert() cleanup

This also contains enhancements to simplify comparisons in name table
publication removal algorithm and to simplify name table sanity checking
when shutting down TIPC.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Per Liden <per.liden@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 94 insertions, 85 deletions

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 051143648edb..e90dc80cd74a 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -71,7 +71,7 @@ struct sub_seq {
  * @sseq: pointer to dynamically-sized array of sub-sequences of this 'type';
  *        sub-sequences are sorted in ascending order
  * @alloc: number of sub-sequences currently in array
- * @first_free: upper bound of highest sub-sequence + 1
+ * @first_free: array index of first unused sub-sequence entry
  * @ns_list: links to adjacent name sequences in hash chain
  * @subscriptions: list of subscriptions for this 'type'
  * @lock: spinlock controlling access to name sequence structure
@@ -175,7 +175,7 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
         nseq->lock = SPIN_LOCK_UNLOCKED;
         nseq->type = type;
         nseq->sseqs = sseq;
-        dbg("tipc_nameseq_create() nseq = %x type %u, ssseqs %x, ff: %u\n",
+        dbg("tipc_nameseq_create(): nseq = %p, type %u, ssseqs %p, ff: %u\n",
             nseq, type, nseq->sseqs, nseq->first_free);
         nseq->alloc = 1;
         INIT_HLIST_NODE(&nseq->ns_list);
@@ -253,16 +253,16 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
         struct sub_seq *sseq;
         int created_subseq = 0;
 
-        assert(nseq->first_free <= nseq->alloc);
         sseq = nameseq_find_subseq(nseq, lower);
-        dbg("nameseq_ins: for seq %x,<%u,%u>, found sseq %x\n",
+        dbg("nameseq_ins: for seq %p, {%u,%u}, found sseq %p\n",
             nseq, type, lower, sseq);
         if (sseq) {
 
                 /* Lower end overlaps existing entry => need an exact match */
 
                 if ((sseq->lower != lower) || (sseq->upper != upper)) {
-                        warn("Overlapping publ <%u,%u,%u>\n", type, lower, upper);
+                        warn("Cannot publish {%u,%u,%u}, overlap error\n",
+                             type, lower, upper);
                         return NULL;
                 }
         } else {
@@ -277,7 +277,8 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
 
                 if ((inspos < nseq->first_free) &&
                     (upper >= nseq->sseqs[inspos].lower)) {
-                        warn("Overlapping publ <%u,%u,%u>\n", type, lower, upper);
+                        warn("Cannot publish {%u,%u,%u}, overlap error\n",
+                             type, lower, upper);
                         return NULL;
                 }
 
@@ -287,7 +288,8 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
                         struct sub_seq *sseqs = tipc_subseq_alloc(nseq->alloc * 2);
 
                         if (!sseqs) {
-                                warn("Memory squeeze; failed to create sub-sequence\n");
+                                warn("Cannot publish {%u,%u,%u}, no memory\n",
+                                     type, lower, upper);
                                 return NULL;
                         }
                         dbg("Allocated %u more sseqs\n", nseq->alloc);
@@ -311,7 +313,7 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
                 sseq->upper = upper;
                 created_subseq = 1;
         }
-        dbg("inserting (%u %u %u) from %x:%u into sseq %x(%u,%u) of seq %x\n",
+        dbg("inserting {%u,%u,%u} from <0x%x:%u> into sseq %p(%u,%u) of seq %p\n",
             type, lower, upper, node, port, sseq,
             sseq->lower, sseq->upper, nseq);
 
@@ -320,7 +322,7 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
         publ = publ_create(type, lower, upper, scope, node, port, key);
         if (!publ)
                 return NULL;
-        dbg("inserting publ %x, node=%x publ->node=%x, subscr->node=%x\n",
+        dbg("inserting publ %p, node=0x%x publ->node=0x%x, subscr->node=%p\n",
             publ, node, publ->node, publ->subscr.node);
 
         if (!sseq->zone_list)
@@ -367,45 +369,47 @@ static struct publication *tipc_nameseq_insert_publ(struct name_seq *nseq,
 
 /**
  * tipc_nameseq_remove_publ -
+ * 
+ * NOTE: There may be cases where TIPC is asked to remove a publication
+ * that is not in the name table.  For example, if another node issues a
+ * publication for a name sequence that overlaps an existing name sequence
+ * the publication will not be recorded, which means the publication won't
+ * be found when the name sequence is later withdrawn by that node.
+ * A failed withdraw request simply returns a failure indication and lets the
+ * caller issue any error or warning messages associated with such a problem.
  */
 
 static struct publication *tipc_nameseq_remove_publ(struct name_seq *nseq, u32 inst,
                                                     u32 node, u32 ref, u32 key)
 {
         struct publication *publ;
+        struct publication *curr;
         struct publication *prev;
         struct sub_seq *sseq = nameseq_find_subseq(nseq, inst);
         struct sub_seq *free;
         struct subscription *s, *st;
         int removed_subseq = 0;
 
-        assert(nseq);
-
-        if (!sseq) {
-                int i;
-
-                warn("Withdraw unknown <%u,%u>?\n", nseq->type, inst);
-                assert(nseq->sseqs);
-                dbg("Dumping subseqs %x for %x, alloc = %u,ff=%u\n",
-                    nseq->sseqs, nseq, nseq->alloc, 
-                    nseq->first_free);
-                for (i = 0; i < nseq->first_free; i++) {
-                        dbg("Subseq %u(%x): lower = %u,upper = %u\n",
-                            i, &nseq->sseqs[i], nseq->sseqs[i].lower,
-                            nseq->sseqs[i].upper);
-                }
+        if (!sseq)
                 return NULL;
-        }
-        dbg("nameseq_remove: seq: %x, sseq %x, <%u,%u> key %u\n",
+
+        dbg("tipc_nameseq_remove_publ: seq: %p, sseq %p, {%u,%u}, key %u\n",
             nseq, sseq, nseq->type, inst, key);
 
+        /* Remove publication from zone scope list */
+
         prev = sseq->zone_list;
         publ = sseq->zone_list->zone_list_next;
         while ((publ->key != key) || (publ->ref != ref) || 
                (publ->node && (publ->node != node))) {
                 prev = publ;
                 publ = publ->zone_list_next;
-                assert(prev != sseq->zone_list);
+                if (prev == sseq->zone_list) {
+                        
+                        /* Prevent endless loop if publication not found */
+
+                        return NULL;
+                }
         }
         if (publ != sseq->zone_list)
                 prev->zone_list_next = publ->zone_list_next;
@@ -416,14 +420,24 @@ static struct publication *tipc_nameseq_remove_publ(struct name_seq *nseq, u32 i
                 sseq->zone_list = NULL;
         }
 
+        /* Remove publication from cluster scope list, if present */
+
         if (in_own_cluster(node)) {
                 prev = sseq->cluster_list;
-                publ = sseq->cluster_list->cluster_list_next;
-                while ((publ->key != key) || (publ->ref != ref) || 
-                       (publ->node && (publ->node != node))) {
-                        prev = publ;
-                        publ = publ->cluster_list_next;
-                        assert(prev != sseq->cluster_list);
+                curr = sseq->cluster_list->cluster_list_next;
+                while (curr != publ) {
+                        prev = curr;
+                        curr = curr->cluster_list_next;
+                        if (prev == sseq->cluster_list) {
+
+                                /* Prevent endless loop for malformed list */
+
+                                err("Unable to de-list cluster publication\n"
+                                    "{%u%u}, node=0x%x, ref=%u, key=%u)\n",
+                                    publ->type, publ->lower, publ->node, 
+                                    publ->ref, publ->key);
+                                goto end_cluster;
+                        }
                 }
                 if (publ != sseq->cluster_list)
                         prev->cluster_list_next = publ->cluster_list_next;
@@ -434,15 +448,26 @@ static struct publication *tipc_nameseq_remove_publ(struct name_seq *nseq, u32 i
                         sseq->cluster_list = NULL;
                 }
         }
+end_cluster:
+
+        /* Remove publication from node scope list, if present */
 
         if (node == tipc_own_addr) {
                 prev = sseq->node_list;
-                publ = sseq->node_list->node_list_next;
-                while ((publ->key != key) || (publ->ref != ref) || 
-                       (publ->node && (publ->node != node))) {
-                        prev = publ;
-                        publ = publ->node_list_next;
-                        assert(prev != sseq->node_list);
+                curr = sseq->node_list->node_list_next;
+                while (curr != publ) {
+                        prev = curr;
+                        curr = curr->node_list_next;
+                        if (prev == sseq->node_list) {
+
+                                /* Prevent endless loop for malformed list */
+
+                                err("Unable to de-list node publication\n"
+                                    "{%u%u}, node=0x%x, ref=%u, key=%u)\n",
+                                    publ->type, publ->lower, publ->node, 
+                                    publ->ref, publ->key);
+                                goto end_node;
+                        }
                 }
                 if (publ != sseq->node_list)
                         prev->node_list_next = publ->node_list_next;
@@ -453,22 +478,18 @@ static struct publication *tipc_nameseq_remove_publ(struct name_seq *nseq, u32 i
                         sseq->node_list = NULL;
                 }
         }
-        assert(!publ->node || (publ->node == node));
-        assert(publ->ref == ref);
-        assert(publ->key == key);
+end_node:
 
-        /* 
-         * Contract subseq list if no more publications:
-         */
-        if (!sseq->node_list && !sseq->cluster_list && !sseq->zone_list) {
+        /* Contract subseq list if no more publications for that subseq */
+
+        if (!sseq->zone_list) {
                 free = &nseq->sseqs[nseq->first_free--];
                 memmove(sseq, sseq + 1, (free - (sseq + 1)) * sizeof (*sseq));
                 removed_subseq = 1;
         }
 
-        /* 
-         * Any subscriptions waiting ? 
-         */
+        /* Notify any waiting subscriptions */
+
         list_for_each_entry_safe(s, st, &nseq->subscriptions, nameseq_list) {
                 tipc_subscr_report_overlap(s,
                                            publ->lower,
@@ -478,6 +499,7 @@ static struct publication *tipc_nameseq_remove_publ(struct name_seq *nseq, u32 i
                                            publ->node,
                                            removed_subseq);
         }
+
         return publ;
 }
 
@@ -530,7 +552,7 @@ static struct name_seq *nametbl_find_seq(u32 type)
         seq_head = &table.types[hash(type)];
         hlist_for_each_entry(ns, seq_node, seq_head, ns_list) {
                 if (ns->type == type) {
-                        dbg("found %x\n", ns);
+                        dbg("found %p\n", ns);
                         return ns;
                 }
         }
@@ -543,22 +565,21 @@ struct publication *tipc_nametbl_insert_publ(u32 type, u32 lower, u32 upper,
 {
         struct name_seq *seq = nametbl_find_seq(type);
 
-        dbg("ins_publ: <%u,%x,%x> found %x\n", type, lower, upper, seq);
+        dbg("tipc_nametbl_insert_publ: {%u,%u,%u} found %p\n", type, lower, upper, seq);
         if (lower > upper) {
-                warn("Failed to publish illegal <%u,%u,%u>\n",
+                warn("Failed to publish illegal {%u,%u,%u}\n",
                      type, lower, upper);
                 return NULL;
         }
 
-        dbg("Publishing <%u,%u,%u> from %x\n", type, lower, upper, node);
+        dbg("Publishing {%u,%u,%u} from 0x%x\n", type, lower, upper, node);
         if (!seq) {
                 seq = tipc_nameseq_create(type, &table.types[hash(type)]);
-                dbg("tipc_nametbl_insert_publ: created %x\n", seq);
+                dbg("tipc_nametbl_insert_publ: created %p\n", seq);
         }
         if (!seq)
                 return NULL;
 
-        assert(seq->type == type);
         return tipc_nameseq_insert_publ(seq, type, lower, upper,
                                         scope, node, port, key);
 }
@@ -572,7 +593,7 @@ struct publication *tipc_nametbl_remove_publ(u32 type, u32 lower,
         if (!seq)
                 return NULL;
 
-        dbg("Withdrawing <%u,%u> from %x\n", type, lower, node);
+        dbg("Withdrawing {%u,%u} from 0x%x\n", type, lower, node);
         publ = tipc_nameseq_remove_publ(seq, lower, node, ref, key);
 
         if (!seq->first_free && list_empty(&seq->subscriptions)) {
@@ -743,7 +764,7 @@ struct publication *tipc_nametbl_publish(u32 type, u32 lower, u32 upper,
                 return NULL;
         }
         if ((type < TIPC_RESERVED_TYPES) && !atomic_read(&rsv_publ_ok)) {
-                warn("Failed to publish reserved name <%u,%u,%u>\n",
+                warn("Publication failed, reserved name {%u,%u,%u}\n",
                      type, lower, upper);
                 return NULL;
         }
@@ -767,10 +788,10 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
 {
         struct publication *publ;
 
-        dbg("tipc_nametbl_withdraw:<%d,%d,%d>\n", type, lower, key);
+        dbg("tipc_nametbl_withdraw: {%u,%u}, key=%u\n", type, lower, key);
         write_lock_bh(&tipc_nametbl_lock);
         publ = tipc_nametbl_remove_publ(type, lower, tipc_own_addr, ref, key);
-        if (publ) {
+        if (likely(publ)) {
                 table.local_publ_count--;
                 if (publ->scope != TIPC_NODE_SCOPE)
                         tipc_named_withdraw(publ);
@@ -780,6 +801,9 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
                 return 1;
         }
         write_unlock_bh(&tipc_nametbl_lock);
+        err("Unable to remove local publication\n"
+            "(type=%u, lower=%u, ref=%u, key=%u)\n",
+            type, lower, ref, key);
         return 0;
 }
 
@@ -787,8 +811,7 @@ int tipc_nametbl_withdraw(u32 type, u32 lower, u32 ref, u32 key)
  * tipc_nametbl_subscribe - add a subscription object to the name table
  */
 
-void
-tipc_nametbl_subscribe(struct subscription *s)
+void tipc_nametbl_subscribe(struct subscription *s)
 {
         u32 type = s->seq.type;
         struct name_seq *seq;
@@ -800,11 +823,13 @@ tipc_nametbl_subscribe(struct subscription *s)
         }
         if (seq){
                 spin_lock_bh(&seq->lock);
-                dbg("tipc_nametbl_subscribe:found %x for <%u,%u,%u>\n",
+                dbg("tipc_nametbl_subscribe:found %p for {%u,%u,%u}\n",
                     seq, type, s->seq.lower, s->seq.upper);
-                assert(seq->type == type);
                 tipc_nameseq_subscribe(seq, s);
                 spin_unlock_bh(&seq->lock);
+        } else {
+                warn("Failed to create subscription for {%u,%u,%u}\n",
+                     s->seq.type, s->seq.lower, s->seq.upper);
         }
         write_unlock_bh(&tipc_nametbl_lock);
 }
@@ -813,8 +838,7 @@ tipc_nametbl_subscribe(struct subscription *s)
  * tipc_nametbl_unsubscribe - remove a subscription object from name table
  */
 
-void
-tipc_nametbl_unsubscribe(struct subscription *s)
+void tipc_nametbl_unsubscribe(struct subscription *s)
 {
         struct name_seq *seq;
 
@@ -1049,35 +1073,20 @@ int tipc_nametbl_init(void)
 
 void tipc_nametbl_stop(void)
 {
-        struct hlist_head *seq_head;
-        struct hlist_node *seq_node;
-        struct hlist_node *tmp;
-        struct name_seq *seq;
         u32 i;
 
         if (!table.types)
                 return;
 
+        /* Verify name table is empty, then release it */
+
         write_lock_bh(&tipc_nametbl_lock);
         for (i = 0; i < tipc_nametbl_size; i++) {
-                seq_head = &table.types[i];
-                hlist_for_each_entry_safe(seq, seq_node, tmp, seq_head, ns_list) {
-                        struct sub_seq *sseq = seq->sseqs;
-
-                        for (; sseq != &seq->sseqs[seq->first_free]; sseq++) {
-                                struct publication *publ = sseq->zone_list;
-                                assert(publ);
-                                do {
-                                        struct publication *next =
-                                                publ->zone_list_next;
-                                        kfree(publ);
-                                        publ = next;
-                                }
-                                while (publ != sseq->zone_list);
-                        }
-                }
+                if (!hlist_empty(&table.types[i]))
+                        err("tipc_nametbl_stop(): hash chain %u is non-null\n", i);
         }
         kfree(table.types);
         table.types = NULL;
         write_unlock_bh(&tipc_nametbl_lock);
 }
+