From 2c24165bc938f9e685eacc3a899a03e26733cd17 Mon Sep 17 00:00:00 2001
From: Stefan Brass <stefan.brass@informatik.uni-halle.de>
Date: Thu, 10 Oct 2019 17:19:32 +0200
Subject: [PATCH] Removed bug in graph.cpp

---
 .gitignore      |   1 +
 graph/graph.cpp | 192 +++++++++++++++++++++++++++---------------------
 2 files changed, 108 insertions(+), 85 deletions(-)

diff --git a/.gitignore b/.gitignore
index dca569c..b0cd4fa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,6 +40,7 @@ RUN_BENCH__RESULT
 /graph/*.P
 /graph/*.tsv
 /graph/*.csv
+/graph/bench_cost.sql
 /graph_data/*_part.tex
 /graph_data/*_part.sql
 /result/*.tsv
diff --git a/graph/graph.cpp b/graph/graph.cpp
index 7a96e60..00f3e05 100644
--- a/graph/graph.cpp
+++ b/graph/graph.cpp
@@ -183,7 +183,8 @@
 // How many bytes does the bitstring in one element of the adjacency list have?
 //=============================================================================
 
-#define BITSTRING_BYTES 4
+//#define BITSTRING_BYTES 4
+#define BITSTRING_BYTES 32
 
 //=============================================================================
 // Should Assertions be tested or is speed of highest importance?
@@ -537,6 +538,10 @@ class NodeSetElem {
 	// Normal Constructor:
 	NodeSetElem(int start_value)
 	{
+		// Check parameter (last bits are represented in bit array):
+		assert(start_value % (BITSTRING_BYTES * 8) == 0);
+
+		// Initialize attributes:
 		next_  = (NodeSetElem *) 0;
 		start_ = start_value;
 		unsigned char *p = bits_;
@@ -546,7 +551,7 @@ class NodeSetElem {
 
 	// Constructor that copies a given node (except next pointer):
 	NodeSetElem(NodeSetElem *elem) {
-		next_  = (NodeSetElem *) 0;
+		next_  = static_cast<NodeSetElem *>(0);
 		start_ = elem->start_;
 		unsigned char *to   = bits_;
 		unsigned char *from = elem->bits_;
@@ -741,8 +746,8 @@ class NodeSet {
 		assert(n >= 1);
 
 		// Compute position in bitmap list:
-		int start = n / (BITSTRING_BYTES * 8);
 		int index = n % (BITSTRING_BYTES * 8);
+		int start = n - index;
 
 		// Handle empty list:
 		if(list_ == NODE_SET_ELEM_NULL) {
@@ -800,8 +805,8 @@ class NodeSet {
 		assert(n >= 1);
 
 		// Compute position in bitmap list:
-		int start = n / (BITSTRING_BYTES * 8);
 		int index = n % (BITSTRING_BYTES * 8);
+		int start = n - index;
 
 		// Search list element with requested start value:
 		for(node_set_elem_t elem = list_; elem; elem = elem->next()) {
@@ -833,29 +838,52 @@ class NodeSet {
 		// own free list, but directly deletes any superfluous nodes.
 		// With the current usage, this will anyway not happen.
 
-		// List of nodes to be used:
+		// List of nodes to be used (recycled):
 		node_set_elem_t nodes = list_;
 
-		// prev is the previous element of the list that is built.
-		// If it is null, then the start (attribute list_) must be set.
-		node_set_elem_t prev = NODE_SET_ELEM_NULL;
-
-		// Copy list, using existing nodes:
+		// List of nodes to be copied:
 		node_set_elem_t from = node_set->list_;
+
+		// Handle empty list:
+		if(from == NODE_SET_ELEM_NULL) {
+			list_ = NODE_SET_ELEM_NULL;
+			free_list(nodes);
+			return;
+		}
+
+		// First element is treated specially, because no previous one:
+		node_set_elem_t first;
+		if(nodes != NODE_SET_ELEM_NULL) {
+			// Get node from recycle list:
+			first = nodes;
+			nodes = nodes->next();
+
+			// Copy node (except next pointer):
+			first->copy_data(from);
+		}
+		else {
+			first = new NodeSetElem(from);
+		}
+
+		// This is the start of the list:
+		list_ = first;
+		node_set_elem_t prev = first;
+
+		// First element was copied:
+		from = from->next();
+
+		// Copy rest of list, using existing nodes as long as possible:
 		for( ; from && nodes; from = from->next()) {
 
 			// Get node from recycle list:
 			node_set_elem_t elem = nodes;
-				nodes = nodes->next();
+			nodes = nodes->next();
 
 			// Copy node (except next pointer):
 			elem->copy_data(from);
 
 			// Link new element to constructed list:
-			if(prev == NODE_SET_ELEM_NULL)
-				list_ = elem;
-			else
-				prev->set_next(elem);
+			prev->set_next(elem);
 
 			// The copied node is now the previous node:
 			prev = elem;
@@ -866,10 +894,7 @@ class NodeSet {
 			node_set_elem_t elem = new NodeSetElem(from);
 
 			// Link new element to "to" list (via prev):
-			if(prev == NODE_SET_ELEM_NULL)
-				list_ = elem;
-			else
-				prev->set_next(elem);
+			prev->set_next(elem);
 
 			// The previous element pointer is always one behind:
 			prev = elem;
@@ -916,11 +941,11 @@ class NodeSetScan {
 	// Constructor:
 	NodeSetScan(NodeSet *node_set)
 	{
-		// Access is permitted by friend declaration in NodeSet:
+		// Remember node set for possible reset of scan:
 		node_set_ = node_set;
 
 		// Set current bitmap list elem:
-		curr_elem_  = node_set_->list_;
+		curr_elem_  = node_set_->list_; // Permitted by friend decl.
 		if(curr_elem_ != NODE_SET_ELEM_NULL)
 			start_ = curr_elem_->start();
 		else
@@ -930,7 +955,7 @@ class NodeSetScan {
 		next_index_ = 0;
 	}
 
-	// Get next node number in set (-1 if there is no further element):
+	// Get next node number in set (NODE_NULL if there is no further one):
 	node_t next() {
 		// Check whether we are at the end of the list:
 		if(curr_elem_ == NODE_SET_ELEM_NULL)
@@ -952,7 +977,7 @@ class NodeSetScan {
 
 		// We know that list elements are non-empty:
 		start_ = curr_elem_->start();
-		int bit = curr_elem_->next_bit(next_index_);
+		int bit = curr_elem_->next_bit(0);
 		next_index_ = bit + 1;
 		return start_ + bit;
 	}
@@ -1005,6 +1030,12 @@ class Graph {
 		changed_   = false;
 	}
 
+	// Destructor:
+	~Graph() {
+		delete[] incoming_;
+		delete[] outgoing_;
+	}
+
 	// Accessor Functions:
 	inline int num_nodes() const { return num_nodes_; }
 
@@ -1086,10 +1117,10 @@ class Graph {
 	// Copy a graph (of the same size) into this graph:
 	void copy(Graph *g)
 	{
-		if(g->num_nodes_ != num_nodes_) {
-			std::cerr << "Can copy only graph of same size!\n";
-			exit(10);
-		}
+		// We can copy only graphs of the same size:
+		assert(g->num_nodes_ == num_nodes_);
+
+		// Copy bit array lists:
 		for(int i = 1; i <= num_nodes_; i++) {
 			incoming_[i].copy(&(g->incoming_[i]));
 			outgoing_[i].copy(&(g->outgoing_[i]));
@@ -1136,7 +1167,7 @@ class TC {
 
 		// Compute transitive closure. First nonrecursive rule:
 		bool changed = false;
-		std::cout << "Iteration 0.\n";
+		std::cout << "\tIteration 0.\n";
 		{ // xs is used locally in this rule (avoids shadowing warning)
 			// tc(X, Y) :- input(X,Y).
 			NodeSetScan xs(input_->out_nodes());
@@ -1154,7 +1185,7 @@ class TC {
 
 		// Naive iteration:
 		while(changed) {
-			std::cout << "Iteration " << (iter_ + 1) << ".\n";
+			std::cout << "\tIteration " << (iter_ + 1) << ".\n";
 			changed = false;
 			tc_prev_->copy(tc_);
 			iter_++;
@@ -1176,7 +1207,7 @@ class TC {
 		}
 
 		// Computation of Rule Instances:
-		std::cout << "Computation of Rule Instances ...\n";
+		std::cout << "\tComputation of Rule Instances ...\n";
 		// Instances of the non-recursive rule were counted above.
 		NodeSetScan xs(input_->out_nodes());
 		NODE_LOOP(x, xs) {
@@ -1261,7 +1292,7 @@ class SG {
 
 		// Compute same generation cousins:
 		bool changed = false;
-		std::cout << "Iteration 0.\n";
+		std::cout << "\tIteration 0.\n";
 		// sg(Y, Y) :- input(X, Y).
 		{ // xs is used locally in this rule (avoids shadowing warning)
 			NodeSetScan xs(input_->out_nodes());
@@ -1284,7 +1315,7 @@ class SG {
 		
 		// Naive iteration:
 		while(changed) {
-			std::cout << "Iteration " << (iter_ + 1) << ".\n";
+			std::cout << "\tIteration " << (iter_ + 1) << ".\n";
 			changed = false;
 			rec_inst = 0;
 			sg_prev_->copy(sg_);
@@ -1416,7 +1447,7 @@ class Output {
 		// Check that there is still space in the array:
 		if(num_files_ >= MAX_OUT_FILES) {
 			std::cerr << "Too many output files!\n";
-			exit(11);
+			exit(3);
 		}
 
 		// Check file extension:
@@ -1438,7 +1469,7 @@ class Output {
 		else {
 			std::cerr << "Output file '" << filename <<
 				"' has unknown extension.\n";
-			exit(12);
+			exit(4);
 		}
 
 		// Open file:
@@ -1447,7 +1478,7 @@ class Output {
 		if(output->fail()) {
 			std::cerr << "Opening output file '" << filename <<
 				"' failed.\n";
-			exit(13);
+			exit(5);
 		}
 		num_files_++;
 	}
@@ -1455,12 +1486,9 @@ class Output {
 	// Set number of nodes:
 	void set_nodes(int n) {
 
-		// The graph data structure depends on the number of nodes:
-		if(num_nodes_ != 0) {
-			std::cerr << "Number of nodes in output can be set " <<
-					"only once!\n";
-			exit(14);
-		}
+		// The graph data structure depends on the number of nodes.
+		// Therefore, the number of nodes can be set only once:
+		assert(num_nodes_ == 0);
 
 		// Store number of nodes:
 		num_nodes_ = n;
@@ -1472,27 +1500,17 @@ class Output {
 	// Write an edge to output file:
 	void write_edge(int from, int to) {
 
-		// The number of nodes must be set:
-		if(num_nodes_ <= 0) {
-			std::cerr << "The number of nodes must be set first!\n";
-			exit(15);
-		}
+		// The number of nodes must be set first:
+		assert(num_nodes_ > 0);
 
 		// Check validity of parameter values:
-		if(from <= 0 || from > num_nodes_) {
-			std::cerr << "from node: " << from << " invalid.\n";
-			exit(16);
-		}
-		if(to <= 0 || to > num_nodes_) {
-			std::cerr << "to node: " << to << " invalid.\n";
-			exit(17);
-		}
+		assert(from > 0);
+		assert(from <= num_nodes_);
+		assert(to > 0);
+		assert(to <= num_nodes_);
 
 		// Check that there is at least one file open:
-		if(num_files_ == 0) {
-			std::cerr << "No file open.\n";
-			exit(18);
-		}
+		assert(num_files_ > 0);
 
 		// Write edge to every file:
 		for(int i = 0; i < num_files_; i++) {
@@ -1537,7 +1555,7 @@ class Output {
 				std::cerr << "Problem with output file " <<
 					"(Format " <<
 					format_name(format_[i]) << ").\n";
-				exit(19);
+				exit(6);
 			}
 			file.close();
 		}
@@ -1572,7 +1590,7 @@ typedef Output *output_t;
 
 void no_second_par(str_t graph_name) {
 	std::cerr << "Graph '" << graph_name << "' has no second parameter.\n";
-	exit(20);
+	exit(7);
 }
 
 //-----------------------------------------------------------------------------
@@ -1581,7 +1599,7 @@ void no_second_par(str_t graph_name) {
 
 void second_par_missing(str_t graph_name) {
 	std::cerr << "Graph '" << graph_name << "' needs a second parameter.\n";
-	exit(21);
+	exit(8);
 }
 
 //-----------------------------------------------------------------------------
@@ -1592,7 +1610,7 @@ void par_too_small(str_t graph_name, str_t par_name, int min) {
 	std::cerr << "Value of parameter '" << par_name << "' " <<
 		"for Graph '" << graph_name << "' too small: " <<
 		"Minimum " << min << ".\n";
-	exit(22);
+	exit(9);
 }
 
 //-----------------------------------------------------------------------------
@@ -1638,7 +1656,7 @@ int next_prime(int p) {
 		if(p < 0) {
 			std::cerr <<
 				"Overflow in computation of prime number\n";
-			exit(23);
+			exit(10);
 		}
 	}
 	return p;
@@ -1707,7 +1725,7 @@ int rand_until(int n) {
 
 	if(result < 0 || result > n) {
 		std::cerr << "Error in random number generator!\n";
-		exit(24);
+		exit(11);
 	}
 #if TEST_OUTPUT
 	std::cout << "rand_until(" << n << ") = " << result <<
@@ -1935,13 +1953,13 @@ void gen_graph_s(int n, int k, output_t out, gsize_t gsize) {
 	// Check divisibility condition:
 	if(n % (k+1) != 0) {
 		std::cerr << "S[n,k] requires that n is divisible by k+1.\n";
-		exit(25);
+		exit(12);
 	}
 
 	// Check that k is not too large:
 	if(n < (k+1)) {
 		std::cerr << "S[n,k] requires that n > k.\n";
-		exit(26);
+		exit(13);
 	}
 
 	// Set number of nodes:
@@ -2271,7 +2289,7 @@ void gen_graph_u_a(int n, int k, output_t out, gsize_t gsize, bool acyclic) {
 		if(i >= p1 * p2) {
 			std::cerr <<
 				"Number of edges too large in random graph.\n";
-			exit(27);
+			exit(14);
 		}
 
 		// Draw the two nodes of a candidate edge:
@@ -2341,7 +2359,7 @@ void gen_graph_u(int n, int k, output_t out, gsize_t gsize) {
 	if(k > n * n) {
 		std::cerr <<
 			"Impossible number of edges (too large) in U-Graph";
-		exit(28);
+		exit(15);
 	}
 
 	// Call joint procedure for graphs U and A:
@@ -2383,7 +2401,7 @@ void gen_graph_a(int n, int k, output_t out, gsize_t gsize) {
 	if(k > n * (n-1) / 2) {
 		std::cerr <<
 			"Impossible number of edges (too large) in A-Graph";
-		exit(29);
+		exit(16);
 	}
 
 	// Call joint procedure for graphs U and A:
@@ -2410,7 +2428,7 @@ int main(int argc, str_t argv[])
 	if(argc < 3) {
 		std::cerr <<
 			"Usage: ./graph [-atsv] GraphID OutputFile1 ...\n";
-		exit(30);
+		exit(17);
 	}
 
 	// Initialize options:
@@ -2446,7 +2464,7 @@ int main(int argc, str_t argv[])
 				// Unknown option:
 				std::cerr << "Unknown option: " <<
 						"'" << *opts << "'\n";
-				exit(31);
+				exit(18);
 			}
 		}
 	}
@@ -2454,14 +2472,14 @@ int main(int argc, str_t argv[])
 	// Get graph parameters, first code:
 	if(argno >= argc) {
 		std::cerr << "Command line argument for Graph ID missing.\n";
-		exit(32);
+		exit(19);
 	}
 	str_t graph_id = argv[argno++];
 	str_t p = graph_id;
 	char graph_code = *p++;
 	if(graph_code == '\0') {
 		std::cerr << "Impossible empty Graph ID.\n";
-		exit(33);
+		exit(20);
 	}
 
 	// First parameter:
@@ -2472,7 +2490,7 @@ int main(int argc, str_t argv[])
 	par_chars[i] = 0;
 	if(i == 0) {
 		std::cerr << "First parameter in graph ID missing.\n";
-		exit(34);
+		exit(21);
 	}
 	int par1 = str_int(par_chars);
 	if(*p == 'k') {
@@ -2507,7 +2525,7 @@ int main(int argc, str_t argv[])
 		par_chars[i] = 0;
 		if(i == 0) {
 			std::cerr << "Second parameter in graph ID missing.\n";
-			exit(35);
+			exit(22);
 		}
 		par2 = str_int(par_chars);
 		if(*p == 'k') {
@@ -2537,13 +2555,17 @@ int main(int argc, str_t argv[])
 	// Check that we have successfully parsed the entire graph ID:
 	if(*p != '\0') {
 		std::cerr << "Unexpected characters at the end of graph ID.\n";
-		exit(36);
+		exit(23);
 	}
 
 	// Some feedback about graph to be generated:
 	std::cout << "Generating graph " << graph_id << " ...\n";
 
 	// Open output file(s):
+	if(argno == argc) {
+		std::cerr << "At least one output file must be specified.\n";
+		exit(24);
+	}
 	Output output;
 	while(argno < argc)
 		output.add_file(argv[argno++]);
@@ -2626,12 +2648,13 @@ int main(int argc, str_t argv[])
 		default:
 			std::cerr << "Unknown graph type '" << graph_code <<
 				"'.\n";
-			exit(35);
+			exit(25);
 	}
 
 	// Output number of edges written:
 	std::cout << "    Number of edges written: " <<
 			output.num_edges() << ".\n";
+	std::cout << "\n";
 
 	// Close output file (in case we interrupt computation):
 	output.close_files();
@@ -2641,9 +2664,9 @@ int main(int argc, str_t argv[])
 
 	// Compute transitive closure data:
 	if(opt_tc) {
-		std::cout << "Compute transitive closure data ...\n";
+		std::cout << "    Compute transitive closure data ...\n";
 		TC tc(output.graph());
-		std::cout << "done\n";
+		std::cout << "\tdone\n";
 		std::cout << "\n";
 
 		// Show computation result:
@@ -2656,9 +2679,9 @@ int main(int argc, str_t argv[])
 
 	// Compute same generation data:
 	if(opt_sg) {
-		std::cout << "Compute same generation data ...\n";
+		std::cout << "    Compute same generation data ...\n";
 		SG sg(output.graph());
-		std::cout << "done\n";
+		std::cout << "\tdone\n";
 		std::cout << "\n";
 
 		// Show computation result:
@@ -2677,7 +2700,6 @@ int main(int argc, str_t argv[])
 
 	// If size data was computed from graph, print that and check result:
 	if(opt_tc || opt_sg) {
-		std::cout << "\n";
 		std::cout << "    Size Data Computed From Graph:\n";
 		//std::cout << "    =============================\n";
 		gsize.print();
@@ -2689,7 +2711,7 @@ int main(int argc, str_t argv[])
 		else {
 			std::cout << "    *** VALUES DIFFER! ***\n";
 			std::cerr << "Inconsistent size values detected!\n";
-			exit(36);
+			exit(26);
 		}
 		std::cout << "\n";
 	}
@@ -2699,7 +2721,7 @@ int main(int argc, str_t argv[])
 	cost_file.open(COST_FILE, std::ios::out|std::ios::app);
 	if(cost_file.fail() || !cost_file.is_open()) {
 		std::cerr << "Opening '" << COST_FILE << "' failed.\n";
-		exit(37);
+		exit(27);
 	}
 
 	// Get tc_size where defined (from formula or computed from graph):
@@ -2750,7 +2772,7 @@ int main(int argc, str_t argv[])
 	cost_file.close();
 	if(cost_file.fail()) {
 		std::cerr << "Closing '" << COST_FILE << "' failed.\n";
-		exit(38);
+		exit(28);
 	}
 
 	// Print elapsed time in seconds:
-- 
GitLab