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.
This commit is contained in:
Darryl L. Miles 2025-02-19 09:58:46 +00:00
parent eb81da6c56
commit 66e72c748a
4 changed files with 36 additions and 36 deletions

View File

@ -58,7 +58,7 @@ extern int CalmaPathCount;
extern HashTable calmaDefInitHash; extern HashTable calmaDefInitHash;
extern void calmaLayerError(char *mesg, int layer, int dt); 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]; plane = cifCurReadPlanes[ciftype];
/* Read the path itself, building up a path structure */ /* 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) if (plane != NULL)
CalmaReadError("Error while reading path for boundary/box; ignored.\n"); CalmaReadError("Error while reading path for boundary/box; ignored.\n");
@ -595,7 +596,8 @@ calmaElementPath(void)
/* Read the points in the path */ /* Read the points in the path */
savescale = calmaReadScale1; savescale = calmaReadScale1;
if (!calmaReadPath(&pathheadp, 2)) pathheadp = calmaReadPath(2);
if (pathheadp == NULL)
{ {
CalmaReadError("Improper path; ignored.\n"); CalmaReadError("Improper path; ignored.\n");
return; return;
@ -1098,26 +1100,24 @@ calmaElementText(void)
* centerline, to avoid roundoff errors. * centerline, to avoid roundoff errors.
* *
* Results: * Results:
* TRUE is returned if the path was parsed successfully, * non-NULL CIFPath* the caller takes ownership of
* FALSE otherwise. * if the path was parsed successfully, otherwise NULL.
* *
* Side effects: * Side effects:
* Modifies the parameter pathheadpp to point to the path * None
* that is constructed.
* *
* ---------------------------------------------------------------------------- * ----------------------------------------------------------------------------
*/ */
bool CIFPath *
calmaReadPath( calmaReadPath(
CIFPath **pathheadpp,
int iscale) int iscale)
{ {
CIFPath path, *pathtailp, *newpathp; CIFPath path, *pathheadp, *pathtailp, *newpathp;
int nbytes, rtype, npoints, savescale; int nbytes, rtype, npoints, savescale;
bool nonManhattan = FALSE; bool nonManhattan = FALSE;
*pathheadpp = (CIFPath *) NULL; pathheadp = (CIFPath *) NULL;
pathtailp = (CIFPath *) NULL; pathtailp = (CIFPath *) NULL;
path.cifp_next = (CIFPath *) NULL; path.cifp_next = (CIFPath *) NULL;
@ -1126,12 +1126,12 @@ calmaReadPath(
if (nbytes < 0) if (nbytes < 0)
{ {
CalmaReadError("EOF when reading path.\n"); CalmaReadError("EOF when reading path.\n");
return (FALSE); return (NULL);
} }
if (rtype != CALMA_XY) if (rtype != CALMA_XY)
{ {
calmaUnexpected(CALMA_XY, rtype); calmaUnexpected(CALMA_XY, rtype);
return (FALSE); return (NULL);
} }
/* Read this many points (pairs of four-byte integers) */ /* Read this many points (pairs of four-byte integers) */
@ -1142,7 +1142,7 @@ calmaReadPath(
calmaReadPoint(&path.cifp_point, iscale); calmaReadPoint(&path.cifp_point, iscale);
if (savescale != calmaReadScale1) if (savescale != calmaReadScale1)
{ {
CIFPath *phead = *pathheadpp; CIFPath *phead = pathheadp;
int newscale = calmaReadScale1 / savescale; int newscale = calmaReadScale1 / savescale;
while (phead != NULL) while (phead != NULL)
{ {
@ -1157,8 +1157,8 @@ calmaReadPath(
} }
if (FEOF(calmaInputFile)) if (FEOF(calmaInputFile))
{ {
CIFFreePath(*pathheadpp); CIFFreePath(pathheadp);
return (FALSE); return (NULL);
} }
if (iscale != 0) if (iscale != 0)
@ -1166,7 +1166,7 @@ calmaReadPath(
newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath))); newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath)));
*newpathp = path; *newpathp = path;
if (*pathheadpp) if (pathheadp)
{ {
/* /*
* Check that this segment is Manhattan. If not, remember the * Check that this segment is Manhattan. If not, remember the
@ -1187,11 +1187,11 @@ calmaReadPath(
} }
pathtailp->cifp_next = newpathp; pathtailp->cifp_next = newpathp;
} }
else *pathheadpp = newpathp; else pathheadp = newpathp;
pathtailp = newpathp; pathtailp = newpathp;
} }
} }
return (*pathheadpp != NULL); return (pathheadp);
} }
/* /*

View File

@ -687,7 +687,8 @@ CIFParseWire(void)
width /= cifReadScale2; width /= cifReadScale2;
savescale = cifReadScale1; savescale = cifReadScale1;
if (!CIFParsePath(&pathheadp, 2)) pathheadp = CIFParsePath(2);
if (pathheadp == NULL)
{ {
CIFReadError("wire, but improper path; ignored.\n"); CIFReadError("wire, but improper path; ignored.\n");
CIFSkipToSemi(); CIFSkipToSemi();
@ -797,7 +798,8 @@ CIFParsePoly(void)
CIFSkipToSemi(); CIFSkipToSemi();
return FALSE; return FALSE;
} }
if (!CIFParsePath(&pathheadp, 1)) pathheadp = CIFParsePath(1);
if (pathheadp == NULL)
{ {
CIFReadError("polygon, but improper path; ignored.\n"); CIFReadError("polygon, but improper path; ignored.\n");
CIFSkipToSemi(); CIFSkipToSemi();

View File

@ -649,12 +649,11 @@ CIFParsePoint(
* one or more points. * one or more points.
* *
* Results: * Results:
* TRUE is returned if the path was parsed successfully, * non-NULL CIFPath* the caller takes ownership of
* FALSE otherwise. * if the path was parsed successfully, otherwise NULL.
* *
* Side effects: * Side effects:
* Modifies the parameter pathheadpp to point to the path * None
* that is constructed.
* *
* Corrections: * Corrections:
* CIF coordinates are multiplied by 2 to cover the case where * CIF coordinates are multiplied by 2 to cover the case where
@ -666,17 +665,16 @@ CIFParsePoint(
* ---------------------------------------------------------------------------- * ----------------------------------------------------------------------------
*/ */
bool CIFPath *
CIFParsePath( CIFParsePath(
CIFPath **pathheadpp,
int iscale) int iscale)
{ {
CIFPath *pathtailp, *newpathp; CIFPath *pathheadp, *pathtailp, *newpathp;
bool nonManhattan = FALSE; /* diagnostic only */ bool nonManhattan = FALSE; /* diagnostic only */
CIFPath path; CIFPath path;
int savescale; int savescale;
*pathheadpp = NULL; pathheadp = NULL;
pathtailp = NULL; pathtailp = NULL;
path.cifp_next = NULL; path.cifp_next = NULL;
while (TRUE) while (TRUE)
@ -688,12 +686,12 @@ CIFParsePath(
savescale = cifReadScale1; savescale = cifReadScale1;
if (!CIFParsePoint(&path.cifp_point, iscale)) if (!CIFParsePoint(&path.cifp_point, iscale))
{ {
CIFFreePath(*pathheadpp); CIFFreePath(pathheadp);
return FALSE; return NULL;
} }
if (savescale != cifReadScale1) if (savescale != cifReadScale1)
{ {
CIFPath *phead = *pathheadpp; CIFPath *phead = pathheadp;
int newscale = cifReadScale1 / savescale; int newscale = cifReadScale1 / savescale;
while (phead != NULL) while (phead != NULL)
{ {
@ -704,7 +702,7 @@ CIFParsePath(
} }
newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath))); newpathp = (CIFPath *) mallocMagic((unsigned) (sizeof (CIFPath)));
*newpathp = path; *newpathp = path;
if (*pathheadpp) if (pathheadp)
{ {
/* /*
* Check that this segment is Manhattan. If not, remember the * Check that this segment is Manhattan. If not, remember the
@ -721,10 +719,10 @@ CIFParsePath(
} }
pathtailp->cifp_next = newpathp; pathtailp->cifp_next = newpathp;
} }
else *pathheadpp = newpathp; else pathheadp = newpathp;
pathtailp = newpathp; pathtailp = newpathp;
} }
return (*pathheadpp != NULL); return pathheadp;
} }
/* /*

View File

@ -166,7 +166,7 @@ extern bool CIFParseUser(void);
extern bool CIFParseCall(void); extern bool CIFParseCall(void);
extern bool CIFParseTransform(Transform *transformp); extern bool CIFParseTransform(Transform *transformp);
extern bool CIFParseInteger(int *valuep); 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 CIFParsePoint(Point *pointp, int iscale);
extern bool CIFParseSInteger(int *valuep); extern bool CIFParseSInteger(int *valuep);
extern void CIFSkipToSemi(void); extern void CIFSkipToSemi(void);