PBuilder on Softswitch_Addresses adds targets for unconnected pins

[heliosfa]

Creating an issue so that I don't forget about this as I am trying to debug a different issue at the moment.

On softswitch_addresses, where you have a pin that is defined in the XML but not conencted, PBuilder is generating an array of targets and indicating that the disconnected pin has a target.

e.g., in netstress_1mailbox.xml.txt, the the spammer device type has two declared output pins, spamOut and out. Aside from the (dummy) init pin, the receiver device type has a single input pin, spamIn, and a single output pin, update, declared.

One instance of a spammer is created, s0000. spamOut is connected to an input pin, spamIn, on an instance of a receiver, r. out is not connected to any input pins on r.

This results in the generation of the following Output Pin array (note that the 3rd field in the initialiser indicates that we have one target when we should have none):

outPin_t Thread_0_Device_s0000_OutputPins[2] = {{PNULL,&Thread_0_DevTyp_0_OutputPins[0],1,Thread_0_Device_s0000_OutPin_spamOut_Tgts,0,0},{PNULL,&Thread_0_DevTyp_0_OutputPins[1],1,Thread_0_Device_s0000_OutPin_out_Tgts,0,0}};

And the generation of the following target arrays:

outEdge_t Thread_0_Device_s0000_OutPin_spamOut_Tgts[1] = {{PNULL,32,0,1}};
outEdge_t Thread_0_Device_s0000_OutPin_out_Tgts[1] = {{PNULL,32,0,1}}; 

The second array is for the disconnected pin and it indicates that the target is the spamIn pin on r. This is potentially problematic as the two pins have different message types - if out ever sends, it will be sending junk to r's spamIn pin.

[heliosfa]

OK, so this looks to be an interplay between _PDIGRAPH::FindArcs not clearing the vector it stores its results in if it doesn't find the pin or any arcs and the way it is used in P_builder.

Specifically, line 1712 of P_builder.cpp uses FindArcs and ignores the return code. outPinArcs is reused between successive pins, so if a pin with no connections follows a pin with connections, outPinArcs is not modified. The check on line 1714 returns false as it still contains the arcs from the connected pin, and these are then replicated in the XML for the disconnected pin. This bug is present in development as well as it uses the same construct.

Three ways forward that I can see:

• Clear the outPinArcs vector before/after every use.
• Modify P_builder to check the return code rather than the size of the array, e.g. remove line 1712 and modify line 1714 to:

  if ( !((*device)->par->G.FindArcs((*device)->idx,(*pin)->idx,inPinArcs,outPinArcs)) ) 

  However, this does not distinguish between an error (the pin was not found) and an expected occurrence (a pin having no connections)

• Modify pdigraph.tpp to clear the vectors (and possibly then return true) in the case that the pin is found but it has no arcs. The current behaviour is as per the documentation but it seems to be inconsistent between the different forms of FindArcs: e.g. The version from line 277 clears the vectors at the start of the call irrespective of whether the node is found or not and whether the node has connections or not.

What do you think @mvousden?.

[mvousden]

I think a combination of your second (check error code) and third (modify pdigraph.tpp) points is the correct solution.

As a check-understanding exercise, I think this patch would solve the problem of point 3:

index a5c84eb..37c469c 100644
--- a/Generics/pdigraph.tpp
+++ b/Generics/pdigraph.tpp
@@ -256,10 +256,10 @@ PDIGRAPH_ bool  _PDIGRAPH::FindArcs(const NKT & n_k,const PKT & p_k,
                                    vector<AKT> & vI,vector<AKT> & vO)
 // Given a node and a pin key, return any/all arc keys
 {
-if (FindNode(n_k)==0) return false;    // Node not there
-if (FindPin(n_k,p_k)==0) return false; // Pin not there
 vI.clear();
 vO.clear();
+if (FindNode(n_k)==0) return false;    // Node not there
+if (FindPin(n_k,p_k)==0) return false; // Pin not there
                                        // Walk right there...
 pair<TPp_it,TPp_it> pi = index_n[n_k].fano.equal_range(p_k);
 for(typename multimap<PKT,pin>::iterator i=pi.first;i!=pi.second;i++)

I'd also ping ADB, since this is his baby.

[heliosfa]

Yes on the patch, that is the quick fix - though I am wondering if "return false if pin found but no connections" is desirable behaviour.