From 66e72c748a75cd964cbebbea92d653f4bcc0071e Mon Sep 17 00:00:00 2001 From: "Darryl L. Miles" Date: Wed, 19 Feb 2025 09:58:46 +0000 Subject: [PATCH] CIFParsePath() prototype change to simply return of created data Previously this was returning two values, a 'bool' and a data structure that is created. Now it simply returns the data structure which makes it easier to reason about who takes ownership of the memory and when, also that no address-of can be supplied that has any side-effect that interacts with how the method works / the returned result. -extern bool CIFParsePath(CIFPath **pathheadpp, int iscale); +extern CIFPath *CIFParsePath(int iscale); Previous related comments: Easier to reason about, there can be no interaction from *pathheadpp and the various functions called, which maybe the first concern to the next reader as visibility of new data is limited to that of a local variable of the function. --- calma/CalmaRdpt.c | 38 +++++++++++++++++++------------------- cif/CIFrdpt.c | 6 ++++-- cif/CIFrdutils.c | 26 ++++++++++++-------------- cif/CIFread.h | 2 +- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/calma/CalmaRdpt.c b/calma/CalmaRdpt.c index f2d1ec71..b23010ab 100644 --- a/calma/CalmaRdpt.c +++ b/calma/CalmaRdpt.c @@ -58,7 +58,7 @@ extern int CalmaPathCount; extern HashTable calmaDefInitHash; extern void calmaLayerError(char *mesg, int layer, int dt); -bool calmaReadPath(CIFPath **pathheadpp, int iscale); +CIFPath *calmaReadPath(int iscale); /* * ---------------------------------------------------------------------------- @@ -237,7 +237,8 @@ calmaElementBoundary(void) plane = cifCurReadPlanes[ciftype]; /* Read the path itself, building up a path structure */ - if (!calmaReadPath(&pathheadp, (plane == NULL) ? 0 : 1)) + pathheadp = calmaReadPath((plane == NULL) ? 0 : 1); + if (pathheadp == NULL) { if (plane != NULL) CalmaReadError("Error while reading path for boundary/box; ignored.\n"); @@ -595,7 +596,8 @@ calmaElementPath(void) /* Read the points in the path */ savescale = calmaReadScale1; - if (!calmaReadPath(&pathheadp, 2)) + pathheadp = calmaReadPath(2); + if (pathheadp == NULL) { CalmaReadError("Improper path; ignored.\n"); return; @@ -1098,26 +1100,24 @@ calmaElementText(void) * centerline, to avoid roundoff errors. * * Results: - * TRUE is returned if the path was parsed successfully, - * FALSE otherwise. + * non-NULL CIFPath* the caller takes ownership of + * if the path was parsed successfully, otherwise NULL. * * Side effects: - * Modifies the parameter pathheadpp to point to the path - * that is constructed. + * None * * ---------------------------------------------------------------------------- */ -bool +CIFPath * calmaReadPath( - CIFPath **pathheadpp, int iscale) { - CIFPath path, *pathtailp, *newpathp; + CIFPath path, *pathheadp, *pathtailp, *newpathp; int nbytes, rtype, npoints, savescale; bool nonManhattan = FALSE; - *pathheadpp = (CIFPath *) NULL; + pathheadp = (CIFPath *) NULL; pathtailp = (CIFPath *) NULL; path.cifp_next = (CIFPath *) NULL; @@ -1126,12 +1126,12 @@ calmaReadPath( if (nbytes < 0) { CalmaReadError("EOF when reading path.\n"); - return (FALSE); + return (NULL); } if (rtype != CALMA_XY) { calmaUnexpected(CALMA_XY, rtype); - return (FALSE); + return (NULL); } /* Read this many points (pairs of four-byte integers) */ @@ -1142,7 +1142,7 @@ calmaReadPath( calmaReadPoint(&path.cifp_point, iscale); if (savescale != calmaReadScale1) { - CIFPath *phead = *pathheadpp; + CIFPath *phead = pathheadp; int newscale = calmaReadScale1 / savescale; while (phead != NULL) { @@ -1157,8 +1157,8 @@ calmaReadPath( } if (FEOF(calmaInputFile)) { - CIFFreePath(*pathheadpp); - return (FALSE); + CIFFreePath(pathheadp); + return (NULL); } if (iscale != 0) @@ -1166,7 +1166,7 @@ calmaReadPath( newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath))); *newpathp = path; - if (*pathheadpp) + if (pathheadp) { /* * Check that this segment is Manhattan. If not, remember the @@ -1187,11 +1187,11 @@ calmaReadPath( } pathtailp->cifp_next = newpathp; } - else *pathheadpp = newpathp; + else pathheadp = newpathp; pathtailp = newpathp; } } - return (*pathheadpp != NULL); + return (pathheadp); } /* diff --git a/cif/CIFrdpt.c b/cif/CIFrdpt.c index 23b8bd86..077740fe 100644 --- a/cif/CIFrdpt.c +++ b/cif/CIFrdpt.c @@ -687,7 +687,8 @@ CIFParseWire(void) width /= cifReadScale2; savescale = cifReadScale1; - if (!CIFParsePath(&pathheadp, 2)) + pathheadp = CIFParsePath(2); + if (pathheadp == NULL) { CIFReadError("wire, but improper path; ignored.\n"); CIFSkipToSemi(); @@ -797,7 +798,8 @@ CIFParsePoly(void) CIFSkipToSemi(); return FALSE; } - if (!CIFParsePath(&pathheadp, 1)) + pathheadp = CIFParsePath(1); + if (pathheadp == NULL) { CIFReadError("polygon, but improper path; ignored.\n"); CIFSkipToSemi(); diff --git a/cif/CIFrdutils.c b/cif/CIFrdutils.c index 23fd9432..31e45da4 100644 --- a/cif/CIFrdutils.c +++ b/cif/CIFrdutils.c @@ -649,12 +649,11 @@ CIFParsePoint( * one or more points. * * Results: - * TRUE is returned if the path was parsed successfully, - * FALSE otherwise. + * non-NULL CIFPath* the caller takes ownership of + * if the path was parsed successfully, otherwise NULL. * * Side effects: - * Modifies the parameter pathheadpp to point to the path - * that is constructed. + * None * * Corrections: * CIF coordinates are multiplied by 2 to cover the case where @@ -666,17 +665,16 @@ CIFParsePoint( * ---------------------------------------------------------------------------- */ -bool +CIFPath * CIFParsePath( - CIFPath **pathheadpp, int iscale) { - CIFPath *pathtailp, *newpathp; + CIFPath *pathheadp, *pathtailp, *newpathp; bool nonManhattan = FALSE; /* diagnostic only */ CIFPath path; int savescale; - *pathheadpp = NULL; + pathheadp = NULL; pathtailp = NULL; path.cifp_next = NULL; while (TRUE) @@ -688,12 +686,12 @@ CIFParsePath( savescale = cifReadScale1; if (!CIFParsePoint(&path.cifp_point, iscale)) { - CIFFreePath(*pathheadpp); - return FALSE; + CIFFreePath(pathheadp); + return NULL; } if (savescale != cifReadScale1) { - CIFPath *phead = *pathheadpp; + CIFPath *phead = pathheadp; int newscale = cifReadScale1 / savescale; while (phead != NULL) { @@ -704,7 +702,7 @@ CIFParsePath( } newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath))); *newpathp = path; - if (*pathheadpp) + if (pathheadp) { /* * Check that this segment is Manhattan. If not, remember the @@ -721,10 +719,10 @@ CIFParsePath( } pathtailp->cifp_next = newpathp; } - else *pathheadpp = newpathp; + else pathheadp = newpathp; pathtailp = newpathp; } - return (*pathheadpp != NULL); + return pathheadp; } /* diff --git a/cif/CIFread.h b/cif/CIFread.h index cb764eb4..2a3a2ac6 100644 --- a/cif/CIFread.h +++ b/cif/CIFread.h @@ -166,7 +166,7 @@ extern bool CIFParseUser(void); extern bool CIFParseCall(void); extern bool CIFParseTransform(Transform *transformp); extern bool CIFParseInteger(int *valuep); -extern bool CIFParsePath(CIFPath **pathheadpp, int iscale); +extern CIFPath *CIFParsePath(int iscale); extern bool CIFParsePoint(Point *pointp, int iscale); extern bool CIFParseSInteger(int *valuep); extern void CIFSkipToSemi(void);