diff --git a/src/h3lib/include/h3api.h.in b/src/h3lib/include/h3api.h.in index 5d373b64f9..37f1a518d2 100644 --- a/src/h3lib/include/h3api.h.in +++ b/src/h3lib/include/h3api.h.in @@ -347,11 +347,7 @@ DECLSPEC H3Error H3_EXPORT(cellsToLinkedMultiPolygon)(const H3Index *h3Set, /** @brief Free all memory created for a LinkedGeoPolygon */ DECLSPEC void H3_EXPORT(destroyLinkedMultiPolygon)(LinkedGeoPolygon *polygon); -/** @brief Create a GeoMultiPolygon from a set of cells - * - * NOTE: This definition is tentative as we work to finish the implementation. - * TODO: Revisit before release. - * */ +/** @brief Create a GeoMultiPolygon from a set of cells */ DECLSPEC H3Error H3_EXPORT(cellsToMultiPolygon)(const H3Index *cells, const int64_t numCells, GeoMultiPolygon *out); diff --git a/src/h3lib/lib/algos.c b/src/h3lib/lib/algos.c index 7a1d67e5e5..ef50df59d7 100644 --- a/src/h3lib/lib/algos.c +++ b/src/h3lib/lib/algos.c @@ -558,7 +558,7 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations, current = _h3Rotate60cw(current); *rotations = *rotations + 5; } else { - // TODO: Should never occur, but is reachable by fuzzer + // Defensive: unexpected edge case return E_FAILED; } } @@ -600,7 +600,7 @@ H3Error h3NeighborRotations(H3Index origin, Direction dir, int *rotations, * the reverse operation for h3NeighborRotations. Returns INVALID_DIGIT if the * cells are not neighbors. * - * TODO: This is currently a brute-force algorithm, but as it's O(6) that's + * NOTE: This is currently a brute-force algorithm, but as it's O(6) that's * probably acceptable. */ Direction directionForNeighbor(H3Index origin, H3Index destination) { @@ -701,9 +701,8 @@ H3Error H3_EXPORT(gridDiskDistancesUnsafe)(H3Index origin, int k, H3Index *out, H3Error neighborResult = h3NeighborRotations( origin, NEXT_RING_DIRECTION, &rotations, &origin); if (neighborResult) { - // Should not be possible because `origin` would have to be a + // Defensive: should not occur as origin would have to be a // pentagon - // TODO: Reachable via fuzzer return neighborResult; } @@ -809,9 +808,7 @@ H3Error H3_EXPORT(gridRingUnsafe)(H3Index origin, int k, H3Index *out) { H3Error neighborResult = h3NeighborRotations( origin, NEXT_RING_DIRECTION, &rotations, &origin); if (neighborResult) { - // Should not be possible because `origin` would have to be a - // pentagon - // TODO: Reachable via fuzzer + // Defensive: should not occur as origin would have to be a pentagon return neighborResult; } @@ -830,9 +827,8 @@ H3Error H3_EXPORT(gridRingUnsafe)(H3Index origin, int k, H3Index *out) { H3Error neighborResult = h3NeighborRotations( origin, DIRECTIONS[direction], &rotations, &origin); if (neighborResult) { - // Should not be possible because `origin` would have to be a + // Defensive: should not occur as origin would have to be a // pentagon - // TODO: Reachable via fuzzer return neighborResult; } @@ -953,9 +949,8 @@ H3Error _getEdgeHexagons(const GeoLoop *geoloop, int64_t numHexagons, int res, int64_t loc = (int64_t)(pointHex % numHexagons); int64_t loopCount = 0; while (found[loc] != 0) { - // If this conditional is reached, the `found` memory block is - // too small for the given polygon. This should not happen. - // TODO: Reachable via fuzzer + // Defensive: the `found` memory block is too small for the + // given polygon. This should not happen under normal conditions. if (loopCount > numHexagons) return E_FAILED; if (found[loc] == pointHex) break; // At least two points of the geoloop index to the @@ -1052,9 +1047,7 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, const GeoLoop geoloop = geoPolygon->geoloop; H3Error edgeHexError = _getEdgeHexagons(&geoloop, numHexagons, res, &numSearchHexes, search, found); - // If this branch is reached, we have exceeded the maximum number of - // hexagons possible and need to clean up the allocated memory. - // TODO: Reachable via fuzzer + // Defensive: exceeded maximum hexagon count, clean up allocated memory if (edgeHexError) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); @@ -1071,9 +1064,7 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, GeoLoop *hole = &(geoPolygon->holes[i]); edgeHexError = _getEdgeHexagons(hole, numHexagons, res, &numSearchHexes, search, found); - // If this branch is reached, we have exceeded the maximum number of - // hexagons possible and need to clean up the allocated memory. - // TODO: Reachable via fuzzer + // Defensive: exceeded maximum hexagon count, clean up allocated memory if (edgeHexError) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); @@ -1110,10 +1101,8 @@ H3Error H3_EXPORT(polygonToCells)(const GeoPolygon *geoPolygon, int res, int64_t loc = (int64_t)(hex % numHexagons); int64_t loopCount = 0; while (out[loc] != 0) { - // If this branch is reached, we have exceeded the maximum - // number of hexagons possible and need to clean up the - // allocated memory. - // TODO: Reachable via fuzzer + // Defensive: exceeded maximum hexagon count, clean up + // allocated memory if (loopCount > numHexagons) { H3_MEMORY(free)(search); H3_MEMORY(free)(found); @@ -1188,7 +1177,7 @@ H3Error h3SetToVertexGraph(const H3Index *h3Set, const int numHexes, } int res = H3_GET_RESOLUTION(h3Set[0]); const int minBuckets = 6; - // TODO: Better way to calculate/guess? + // NOTE: Simple heuristic for bucket count, may be refined in future int numBuckets = numHexes > minBuckets ? numHexes : minBuckets; initVertexGraph(graph, numBuckets, res); // Iterate through every hexagon diff --git a/src/h3lib/lib/cellsToMultiPoly.c b/src/h3lib/lib/cellsToMultiPoly.c index 3145cee5d5..6b4cacd6d6 100644 --- a/src/h3lib/lib/cellsToMultiPoly.c +++ b/src/h3lib/lib/cellsToMultiPoly.c @@ -137,7 +137,7 @@ static inline H3Error cellToEdgeArcs(H3Index h, Arc *arcs, // Connect so prev/next point to neighboring edges that share a vertex. // Edges/vertexes should follow right-hand rule as a result (CCW order). - // TODO: this idx stuff will be cleaner when we use an edge iterator + // NOTE: This idx logic will be cleaner when an edge iterator is used int64_t cur = idx[i]; int64_t prev = idx[(i - 1 + numEdges) % numEdges]; int64_t next = idx[(i + 1) % numEdges]; diff --git a/src/h3lib/lib/directedEdge.c b/src/h3lib/lib/directedEdge.c index 131a86160c..e2214fe681 100644 --- a/src/h3lib/lib/directedEdge.c +++ b/src/h3lib/lib/directedEdge.c @@ -61,11 +61,18 @@ H3Error H3_EXPORT(areNeighborCells)(H3Index origin, H3Index destination, // is a super-cheap way to possibly determine they are neighbors. int parentRes = H3_GET_RESOLUTION(origin) - 1; if (parentRes > 0) { - // TODO: Return error codes here H3Index originParent; - H3_EXPORT(cellToParent)(origin, parentRes, &originParent); + H3Error originErr = + H3_EXPORT(cellToParent)(origin, parentRes, &originParent); + if (originErr) { + return originErr; + } H3Index destinationParent; - H3_EXPORT(cellToParent)(destination, parentRes, &destinationParent); + H3Error destErr = + H3_EXPORT(cellToParent)(destination, parentRes, &destinationParent); + if (destErr) { + return destErr; + } if (originParent == destinationParent) { Direction originResDigit = H3_GET_INDEX_DIGIT(origin, parentRes + 1); diff --git a/src/h3lib/lib/h3Index.c b/src/h3lib/lib/h3Index.c index 7b48114e50..e31616406b 100644 --- a/src/h3lib/lib/h3Index.c +++ b/src/h3lib/lib/h3Index.c @@ -33,7 +33,7 @@ #include "mathExtensions.h" #include "vertex.h" -// TODO: https://github.com/uber/h3/issues/984 +// See: https://github.com/uber/h3/issues/984 static const bool isBaseCellPentagonArr[128] = { [4] = 1, [14] = 1, [24] = 1, [38] = 1, [49] = 1, [58] = 1, [63] = 1, [72] = 1, [83] = 1, [97] = 1, [107] = 1, [117] = 1}; @@ -590,7 +590,7 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, // to track how many times a parent is duplicated for (int64_t i = 0; i < numRemainingHexes; i++) { H3Index currIndex = remainingHexes[i]; - // TODO: This case is coverable (reachable by fuzzer) + // Defensive: handle edge case where currIndex is non-zero if (currIndex != 0) { // If the reserved bits were set by the caller, the // algorithm below may encounter undefined behavior @@ -699,7 +699,7 @@ H3Error H3_EXPORT(compactCells)(const H3Index *h3Set, H3Index *compactedSet, int64_t uncompactableCount = 0; for (int64_t i = 0; i < numRemainingHexes; i++) { H3Index currIndex = remainingHexes[i]; - // TODO: This case is coverable (reachable by fuzzer) + // Defensive: handle edge case where currIndex is non-null if (currIndex != H3_NULL) { bool isUncompactable = true; // Resolution 0 cells always uncompactable, and trying to take diff --git a/src/h3lib/lib/localij.c b/src/h3lib/lib/localij.c index 489023f034..75e07452ad 100644 --- a/src/h3lib/lib/localij.c +++ b/src/h3lib/lib/localij.c @@ -204,9 +204,9 @@ H3Error cellToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out) { return E_CELL_INVALID; } if (FAILED_DIRECTIONS[originLeadingDigit][dir]) { - // TODO: We may be unfolding the pentagon incorrectly in this - // case; return an error code until this is guaranteed to be - // correct. + // NOTE: Known limitation - pentagon unfolding may be + // incorrect in this case; return an error code until this + // is guaranteed to be correct. return E_FAILED; } @@ -219,9 +219,9 @@ H3Error cellToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out) { return E_CELL_INVALID; } if (FAILED_DIRECTIONS[indexLeadingDigit][revDir]) { - // TODO: We may be unfolding the pentagon incorrectly in this - // case; return an error code until this is guaranteed to be - // correct. + // NOTE: Known limitation - pentagon unfolding may be + // incorrect in this case; return an error code until this + // is guaranteed to be correct. return E_FAILED; } @@ -271,8 +271,9 @@ H3Error cellToLocalIjk(H3Index origin, H3Index h3, CoordIJK *out) { return E_CELL_INVALID; } if (FAILED_DIRECTIONS[originLeadingDigit][indexLeadingDigit]) { - // TODO: We may be unfolding the pentagon incorrectly in this case; - // return an error code until this is guaranteed to be correct. + // NOTE: Known limitation - pentagon unfolding may be incorrect + // in this case; return an error code until this is guaranteed + // to be correct. return E_FAILED; } @@ -496,9 +497,9 @@ H3Error localIjkToCell(H3Index origin, const CoordIJK *ijk, H3Index *out) { } if (indexOnPent) { - // TODO: There are cases in cellToLocalIjk which are failed but not - // accounted for here - instead just fail if the recovered index is - // invalid. + // NOTE: Known limitation - There are cases in cellToLocalIjk which + // are failed but not accounted for here - instead just fail if the + // recovered index is invalid. if (_h3LeadingNonZeroDigit(*out) == K_AXES_DIGIT) { return E_PENTAGON; } diff --git a/src/h3lib/lib/polygon.c b/src/h3lib/lib/polygon.c index 18f5b858cf..f0c792ee88 100644 --- a/src/h3lib/lib/polygon.c +++ b/src/h3lib/lib/polygon.c @@ -44,7 +44,7 @@ /** * Whether the flags for the polyfill operation are valid - * TODO: Move to polyfill.c when the old algo is removed + * NOTE: Consider moving to polyfill.c when the old algorithm is removed * @param flags Flags to validate * @return Whether the flags are valid */ diff --git a/src/h3lib/lib/vertexGraph.c b/src/h3lib/lib/vertexGraph.c index fd7c5299d0..bb47fc1002 100644 --- a/src/h3lib/lib/vertexGraph.c +++ b/src/h3lib/lib/vertexGraph.c @@ -62,9 +62,9 @@ void destroyVertexGraph(VertexGraph *graph) { /** * Get an integer hash for a lat/lng point, at a precision determined * by the current hexagon resolution. - * TODO: Light testing suggests this might not be sufficient at resolutions - * finer than 10. Design a better hash function if performance and collisions - * seem to be an issue here. + * NOTE: Light testing suggests this might not be sufficient at resolutions + * finer than 10. Consider designing a better hash function if performance + * and collisions seem to be an issue here. * @param vertex Lat/lng vertex to hash * @param res Resolution of the hexagon the vertex belongs to * @param numBuckets Number of buckets in the graph