From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sun, 19 Dec 2021 21:16:00 +0000 (-0800)
Subject: graph_sgd: track number of nodes as a 'size_t' instead of 'int'
X-Git-Tag: 3.0.0~115^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1c42bb82cd73cb45d3abee845c21819add00d982;p=graphviz

graph_sgd: track number of nodes as a 'size_t' instead of 'int'

This squashes a number of compiler warnings and allows this struct to deal with
> 2³² - 1 nodes in future. Dealing with this number of nodes now is still not
possible due to locations where conversion back to `int` is necessary. In
future the source and target index arrays in this struct should probably be
converted to `size_t` too, but that was not done in this commit as it involves
more invasive changes.
---

diff --git a/lib/neatogen/dijkstra.c b/lib/neatogen/dijkstra.c
index 3f3ae34e7..7b60ce68f 100644
--- a/lib/neatogen/dijkstra.c
+++ b/lib/neatogen/dijkstra.c
@@ -17,6 +17,7 @@
 
 ******************************************/
 
+#include <assert.h>
 #include <common/memory.h>
 #include <float.h>
 #include <neatogen/bfs.h>
@@ -369,16 +370,16 @@ int dijkstra_sgd(graph_sgd *graph, int source, term_sgd *terms) {
     heap h;
     int *indices = N_GNEW(graph->n, int);
     float *dists = N_GNEW(graph->n, float);
-    int i;
-    for (i=0; i<graph->n; i++) {
+    for (size_t i= 0; i < graph->n; i++) {
         dists[i] = FLT_MAX;
     }
     dists[source] = 0;
-    for (i=graph->sources[source]; i<graph->sources[source+1]; i++) {
+    for (int i = graph->sources[source]; i < graph->sources[source + 1]; i++) {
         int target = graph->targets[i];
         dists[target] = graph->weights[i];
     }
-    initHeap_f(&h, source, indices, dists, graph->n);
+    assert(graph->n <= INT_MAX);
+    initHeap_f(&h, source, indices, dists, (int)graph->n);
 
     int closest = 0, offset = 0;
     while (extractMax_f(&h, &closest, indices, dists)) {
@@ -395,7 +396,8 @@ int dijkstra_sgd(graph_sgd *graph, int source, term_sgd *terms) {
             terms[offset].w = 1 / (d*d);
             offset++;
         }
-        for (i=graph->sources[closest]; i<graph->sources[closest+1]; i++) {
+        for (int i = graph->sources[closest]; i < graph->sources[closest + 1];
+             i++) {
             int target = graph->targets[i];
             float weight = graph->weights[i];
             increaseKey_f(&h, target, d+weight, indices, dists);
diff --git a/lib/neatogen/sgd.c b/lib/neatogen/sgd.c
index 6dcf63367..c33c82065 100644
--- a/lib/neatogen/sgd.c
+++ b/lib/neatogen/sgd.c
@@ -1,3 +1,5 @@
+#include <assert.h>
+#include <limits.h>
 #include <neatogen/neato.h>
 #include <neatogen/sgd.h>
 #include <neatogen/dijkstra.h>
@@ -37,7 +39,7 @@ static void fisheryates_shuffle(term_sgd *terms, int n_terms) {
 static graph_sgd * extract_adjacency(graph_t *G, int model) {
     node_t *np;
     edge_t *ep;
-    int n_nodes = 0, n_edges = 0;
+    size_t n_nodes = 0, n_edges = 0;
     for (np = agfstnode(G); np; np = agnxtnode(G,np)) {
         assert(ND_id(np) == n_nodes);
         n_nodes++;
@@ -54,11 +56,13 @@ static graph_sgd * extract_adjacency(graph_t *G, int model) {
     graph->weights = N_NEW(n_edges, float);
 
     graph->n = n_nodes;
-    graph->sources[graph->n] = n_edges; // to make looping nice
+    assert(n_edges <= INT_MAX);
+    graph->sources[graph->n] = (int)n_edges; // to make looping nice
 
     n_nodes = 0, n_edges = 0;
     for (np = agfstnode(G); np; np = agnxtnode(G,np)) {
-        graph->sources[n_nodes] = n_edges;
+        assert(n_edges <= INT_MAX);
+        graph->sources[n_nodes] = (int)n_edges;
         graph->pinneds[n_nodes] = isFixed(np);
         for (ep = agfstedge(G, np); ep; ep = agnxtedge(G, ep, np)) {
             if (agtail(ep) == aghead(ep)) { // ignore self-loops and double edges
@@ -73,22 +77,22 @@ static graph_sgd * extract_adjacency(graph_t *G, int model) {
         n_nodes++;
     }
     assert(n_nodes == graph->n);
-    assert(n_edges == graph->sources[graph->n]);
-    graph->sources[n_nodes] = n_edges;
+    assert(n_edges <= INT_MAX);
+    assert((int)n_edges == graph->sources[graph->n]);
+    graph->sources[n_nodes] = (int)n_edges;
 
     if (model == MODEL_SHORTPATH) {
         // do nothing
     } else if (model == MODEL_SUBSET) {
         // i,j,k refer to actual node indices, while x,y refer to edge indices in graph->targets
-        int i;
         bool *neighbours_i = N_NEW(graph->n, bool);
         bool *neighbours_j = N_NEW(graph->n, bool);
-        for (i=0; i<graph->n; i++) {
+        for (size_t i = 0; i < graph->n; i++) {
             // initialise to no neighbours
             neighbours_i[i] = false;
             neighbours_j[i] = false;
         }
-        for (i=0; i<graph->n; i++) {
+        for (size_t i = 0; i < graph->n; i++) {
             int x;
             int deg_i = 0;
             for (x=graph->sources[i]; x<graph->sources[i+1]; x++) {
diff --git a/lib/neatogen/sgd.h b/lib/neatogen/sgd.h
index feed10a3a..af11526c7 100644
--- a/lib/neatogen/sgd.h
+++ b/lib/neatogen/sgd.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <stddef.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -10,7 +12,7 @@ typedef struct term_sgd {
 } term_sgd;
 
 typedef struct graph_sgd {
-    int n; // number of nodes
+    size_t n; // number of nodes
     int *sources; // index of first edge in *targets for each node (length n+1)
     bool *pinneds; // whether a node is fixed or not