From: Matthew Fernandez Date: Sun, 21 Aug 2022 02:46:02 +0000 (-0700) Subject: API BREAK: libxdot: use size_t instead of int for stats and polygon line points X-Git-Tag: 6.0.1~29^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cb2619f7c65eb9d8b9ad7a1d9e8fe59e014c7e7d;p=graphviz API BREAK: libxdot: use size_t instead of int for stats and polygon line points This allows exceeding counts of 2³¹ - 1, which is not particularly likely. But its more important effect is to make arithmetic operations less error prone and make it impossible by-construction to arrive at a negative count in these fields. There is some interaction between the stats fields and polygon line points fields, so doing these two separately would have meant an intermediate state with lots of casting. It seemed simpler and less error prone to do this all at once. There is still casting back to int for the `gvrender_*` functions. Modifying these would have been too invasive. Surprisingly the call to `gvrender_polyline` affected in this commit is the only instance of a call to it with a non-literal point count. But modifying it to take a size_t would still have required too-invasive changes due to other functions it calls. For lines that would have been changed anyway, this commit also fixes white space and tightens variable scoping where possible. --- diff --git a/CHANGELOG.md b/CHANGELOG.md index f73ea66e6..3a70214fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **Breaking**: libxdot fields for the size and number of operations are now - `size_t` values instead of `int` values +- **Breaking**: libxdot fields for the size and number of operations, the + statistics counts, and polygon line points are now `size_t` values instead of + `int` values ### Removed diff --git a/cmd/smyrna/draw.c b/cmd/smyrna/draw.c index 300b78cb3..6bfcf6bf7 100644 --- a/cmd/smyrna/draw.c +++ b/cmd/smyrna/draw.c @@ -121,7 +121,6 @@ static void set_options(int param) static void DrawBeziers(sdot_op* o, int param) { int filled; - int i = 0; xdot_op * op=&o->op; xdot_point* ps = op->u.bezier.pts; view->Topview->global_z = view->Topview->global_z + o->layer*LAYER_DIFF; @@ -131,7 +130,7 @@ static void DrawBeziers(sdot_op* o, int param) else filled = 0; - for (i = 1; i < op->u.bezier.cnt; i += 3) { + for (size_t i = 1; i < op->u.bezier.cnt; i += 3) { DrawBezier(ps, filled, param); ps += 3; } @@ -219,7 +218,6 @@ static void DrawPolygon(sdot_op * o, int param) static void DrawPolyline(sdot_op* o, int param) { - int i = 0; xdot_op * op=&o->op; view->Topview->global_z=view->Topview->global_z+o->layer*LAYER_DIFF; @@ -232,7 +230,7 @@ static void DrawPolyline(sdot_op* o, int param) set_options(param); glLineWidth(view->LineWidth); glBegin(GL_LINE_STRIP); - for (i = 0; i < op->u.polyline.cnt; i = i + 1) { + for (size_t i = 0; i < op->u.polyline.cnt; ++i) { glVertex3f((GLfloat) op->u.polyline.pts[i].x - dx, (GLfloat) op->u.polyline.pts[i].y - dy, (GLfloat) op->u.polyline.pts[i].z + view->Topview->global_z); diff --git a/cmd/smyrna/polytess.c b/cmd/smyrna/polytess.c index c7d22c09f..ec9796818 100644 --- a/cmd/smyrna/polytess.c +++ b/cmd/smyrna/polytess.c @@ -9,6 +9,7 @@ *************************************************************************/ #include "polytess.h" +#include #include tessPoly TP; @@ -76,16 +77,14 @@ static void Set_Winding_Rule(GLUtesselator *tobj, GLenum winding_rule) static void Render_Contour2(GLUtesselator *tobj, sdot_op* p) { - int x=0; - GLdouble* d = calloc(p->op.u.polygon.cnt * 3, sizeof(GLdouble)); - for (x=0;x < p->op.u.polygon.cnt; x++) + for (size_t x = 0; x < p->op.u.polygon.cnt; x++) { d[x * 3] = p->op.u.polygon.pts[x].x; d[x * 3 + 1] = p->op.u.polygon.pts[x].y; d[x * 3 + 2] = p->op.u.polygon.pts[x].z + view->Topview->global_z; } - for (x = 0; x < p->op.u.polygon.cnt; x++) //loop through the vertices + for (size_t x = 0; x < p->op.u.polygon.cnt; x++) //loop through the vertices { gluTessVertex(tobj, &d[x * 3], &d[x * 3]); //store the vertex } diff --git a/cmd/smyrna/topviewfuncs.c b/cmd/smyrna/topviewfuncs.c index e08cf25fc..2f1c9f2bf 100644 --- a/cmd/smyrna/topviewfuncs.c +++ b/cmd/smyrna/topviewfuncs.c @@ -486,9 +486,9 @@ static char* readPoint (char* p, xdot_point* pt) * return start of point list (skip over e and s points). * return NULL on failure */ -static char* countPoints (char* pos, int* have_sp, xdot_point* sp, int* have_ep, xdot_point* ep, int* cntp) -{ - int cnt = 0; +static char *countPoints(char *pos, int *have_sp, xdot_point *sp, int *have_ep, + xdot_point *ep, size_t *cntp) { + size_t cnt = 0; char* p; pos = skipWS (pos); @@ -553,7 +553,8 @@ static int storePoints (char* pos, xdot_point* ps) static xdot* makeXDotSpline (char* pos) { xdot_point s, e; - int v, have_s, have_e, cnt; + int v, have_s, have_e; + size_t cnt; static const size_t sz = sizeof(sdot_op); xdot* xd; xdot_op* op; diff --git a/lib/common/emit.c b/lib/common/emit.c index c2ea7e0bd..dfc1449e5 100644 --- a/lib/common/emit.c +++ b/lib/common/emit.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -1450,10 +1451,9 @@ static bool write_node_test(Agraph_t * g, Agnode_t * n) #define INITPTS 1000 -static pointf* -copyPts (pointf* pts, int* ptsize, xdot_point* inpts, int numpts) -{ - int i, sz = *ptsize; +static pointf *copyPts(pointf *pts, size_t *ptsize, xdot_point *inpts, + size_t numpts) { + size_t sz = *ptsize; if (numpts > sz) { sz = MAX(2*sz, numpts); @@ -1461,7 +1461,7 @@ copyPts (pointf* pts, int* ptsize, xdot_point* inpts, int numpts) *ptsize = sz; } - for (i = 0; i < numpts; i++) { + for (size_t i = 0; i < numpts; i++) { pts[i].x = inpts[i].x; pts[i].y = inpts[i].y; } @@ -1472,7 +1472,7 @@ copyPts (pointf* pts, int* ptsize, xdot_point* inpts, int numpts) static void emit_xdot (GVJ_t * job, xdot* xd) { int image_warn = 1; - int ptsize = INITPTS; + size_t ptsize = INITPTS; pointf* pts = N_GNEW(INITPTS, pointf); exdot_op* op; int angle; @@ -1496,7 +1496,9 @@ static void emit_xdot (GVJ_t * job, xdot* xd) case xd_unfilled_polygon : if (boxf_overlap(op->bb, job->clip)) { pts = copyPts (pts, &ptsize, op->op.u.polygon.pts, op->op.u.polygon.cnt); - gvrender_polygon(job, pts, op->op.u.polygon.cnt, + assert(op->op.u.polygon.cnt <= INT_MAX && + "polygon count exceeds gvrender_polygon support"); + gvrender_polygon(job, pts, (int)op->op.u.polygon.cnt, op->op.kind == xd_filled_polygon ? filled : 0); } break; @@ -1504,14 +1506,18 @@ static void emit_xdot (GVJ_t * job, xdot* xd) case xd_unfilled_bezier : if (boxf_overlap(op->bb, job->clip)) { pts = copyPts (pts, &ptsize, op->op.u.bezier.pts, op->op.u.bezier.cnt); - gvrender_beziercurve(job, pts, op->op.u.bezier.cnt, 0, 0, + assert(op->op.u.bezier.cnt <= INT_MAX && + "polygon count exceeds gvrender_beizercurve support"); + gvrender_beziercurve(job, pts, (int)op->op.u.bezier.cnt, 0, 0, op->op.kind == xd_filled_bezier ? filled : 0); } break; case xd_polyline : if (boxf_overlap(op->bb, job->clip)) { pts = copyPts (pts, &ptsize, op->op.u.polyline.pts, op->op.u.polyline.cnt); - gvrender_polyline(job, pts, op->op.u.polyline.cnt); + assert(op->op.u.polyline.cnt <= INT_MAX && + "polygon count exceeds gvrender_polyline support"); + gvrender_polyline(job, pts, (int)op->op.u.polyline.cnt); } break; case xd_text : @@ -2872,15 +2878,12 @@ expandBB (boxf* bb, pointf p) bb->LL.y = fmin(bb->LL.y, p.y); } -static boxf -ptsBB (xdot_point* inpts, int numpts, boxf* bb) -{ +static boxf ptsBB(xdot_point *inpts, size_t numpts, boxf *bb) { boxf opbb; - int i; opbb.LL.x = opbb.UR.x = inpts->x; opbb.LL.y = opbb.UR.y = inpts->y; - for (i = 1; i < numpts; i++) { + for (size_t i = 1; i < numpts; i++) { inpts++; if (inpts->x < opbb.LL.x) opbb.LL.x = inpts->x; diff --git a/lib/xdot/xdot.c b/lib/xdot/xdot.c index d32a28fdf..b889755dd 100644 --- a/lib/xdot/xdot.c +++ b/lib/xdot/xdot.c @@ -86,15 +86,14 @@ static char *parseRect(char *s, xdot_rect * rp) static char *parsePolyline(char *s, xdot_polyline * pp) { - int i; + unsigned i; xdot_point *pts; xdot_point *ps; char* endp; - s = parseInt(s, &i); + s = parseUInt(s, &i); if (!s) return NULL; - if (i < 0) return NULL; - pts = ps = gv_calloc((size_t)i, sizeof(ps[0])); + pts = ps = gv_calloc(i, sizeof(ps[0])); pp->cnt = i; for (i = 0; i < pp->cnt; i++) { ps->x = strtod (s, &endp); @@ -434,11 +433,10 @@ static void printRect(xdot_rect * r, pf print, void *info) static void printPolyline(xdot_polyline * p, pf print, void *info) { - int i; char buf[512]; - print(info, " %d", p->cnt); - for (i = 0; i < p->cnt; i++) { + print(info, " %" PRISIZE_T, p->cnt); + for (size_t i = 0; i < p->cnt; i++) { snprintf(buf, sizeof(buf), " %.02f", p->pts[i].x); trim(buf); print(info, "%s", buf); @@ -610,10 +608,8 @@ static void jsonRect(xdot_rect * r, pf print, void *info) static void jsonPolyline(xdot_polyline * p, pf print, void *info) { - int i; - print(info, "["); - for (i = 0; i < p->cnt; i++) { + for (size_t i = 0; i < p->cnt; i++) { print(info, "%.06f,%.06f", p->pts[i].x, p->pts[i].y); if (i < p->cnt-1) print(info, ","); } diff --git a/lib/xdot/xdot.h b/lib/xdot/xdot.h index d681774ce..f226bdb9c 100644 --- a/lib/xdot/xdot.h +++ b/lib/xdot/xdot.h @@ -78,7 +78,7 @@ typedef struct { } xdot_rect; typedef struct { - int cnt; + size_t cnt; xdot_point* pts; } xdot_polyline; @@ -153,20 +153,20 @@ typedef struct { typedef struct { size_t cnt; /* no. of xdot ops */ - int n_ellipse; - int n_polygon; - int n_polygon_pts; - int n_polyline; - int n_polyline_pts; - int n_bezier; - int n_bezier_pts; - int n_text; - int n_font; - int n_style; - int n_color; - int n_image; - int n_gradcolor; - int n_fontchar; + size_t n_ellipse; + size_t n_polygon; + size_t n_polygon_pts; + size_t n_polyline; + size_t n_polyline_pts; + size_t n_bezier; + size_t n_bezier_pts; + size_t n_text; + size_t n_font; + size_t n_style; + size_t n_color; + size_t n_image; + size_t n_gradcolor; + size_t n_fontchar; } xdot_stats; /* ops are indexed by xop_kind */ diff --git a/plugin/core/gvrender_core_json.c b/plugin/core/gvrender_core_json.c index df2c78917..921db8fbc 100644 --- a/plugin/core/gvrender_core_json.c +++ b/plugin/core/gvrender_core_json.c @@ -158,12 +158,11 @@ static void set_attrwf(Agraph_t * g, bool toplevel, bool value) static void write_polyline (GVJ_t * job, xdot_polyline* polyline) { - int i; - int cnt = polyline->cnt; + const size_t cnt = polyline->cnt; xdot_point* pts = polyline->pts; gvprintf(job, "\"points\": ["); - for (i = 0; i < cnt; i++) { + for (size_t i = 0; i < cnt; i++) { if (i > 0) gvprintf(job, ","); gvprintf(job, "[%.03f,%.03f]", pts[i].x, pts[i].y); }