From 83fea2a91bad18405dbd629c1daf6c85f82d53f5 Mon Sep 17 00:00:00 2001 From: Isaac Brodsky Date: Sun, 20 Feb 2022 10:09:16 -0800 Subject: [PATCH] Add error return code to cellToCenterChild --- CHANGELOG.md | 2 ++ src/apps/benchmarks/benchmarkIsValidCell.c | 5 +++-- src/apps/fuzzers/fuzzerHierarchy.c | 4 ++-- src/apps/testapps/testCellToCenterChild.c | 21 ++++++++++++++------- src/apps/testapps/testCompactCells.c | 3 ++- src/h3lib/include/h3api.h.in | 3 ++- src/h3lib/lib/h3Index.c | 13 ++++++------- website/docs/core-library/usage.md | 5 ++++- 8 files changed, 35 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3ed98b397..0682a06806 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ The public API of this library consists of the functions declared in file [h3api.h.in](./src/h3lib/include/h3api.h.in). ## [Unreleased] +### Breaking changes +- `cellToCenterChild` (previously `h3ToCenterChild`) returns an error code. (#581) ## [4.0.0-rc1] - 2022-02-07 ### Breaking changes diff --git a/src/apps/benchmarks/benchmarkIsValidCell.c b/src/apps/benchmarks/benchmarkIsValidCell.c index 23e747e3b3..2e1e37dd32 100644 --- a/src/apps/benchmarks/benchmarkIsValidCell.c +++ b/src/apps/benchmarks/benchmarkIsValidCell.c @@ -27,8 +27,9 @@ CellArray pentagonSetup(int parentRes, int childRes, int nullEvery) { // // If `nullEvery > 0`, then modify the array to have H3_NULL // every `nullEvery` indices. - H3Index p = 0x80c3fffffffffff; // res 0 pentagon - p = H3_EXPORT(cellToCenterChild)(p, parentRes); + H3Index pParent = 0x80c3fffffffffff; // res 0 pentagon + H3Index p; + H3_EXPORT(cellToCenterChild)(pParent, parentRes, &p); CellArray ca; diff --git a/src/apps/fuzzers/fuzzerHierarchy.c b/src/apps/fuzzers/fuzzerHierarchy.c index 67ea7e63f2..490e23878b 100644 --- a/src/apps/fuzzers/fuzzerHierarchy.c +++ b/src/apps/fuzzers/fuzzerHierarchy.c @@ -38,8 +38,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { H3Index parent; H3_EXPORT(cellToParent)(args->index, args->parentRes, &parent); - // TODO: Update with new API - H3_EXPORT(cellToCenterChild)(args->index, args->childRes); + H3Index out; + H3_EXPORT(cellToCenterChild)(args->index, args->childRes, &out); int resDiff = args->childRes - H3_EXPORT(getResolution)(args->index); if (resDiff < MAX_CHILDREN_DIFF) { diff --git a/src/apps/testapps/testCellToCenterChild.c b/src/apps/testapps/testCellToCenterChild.c index 980081fdf4..215e657a57 100644 --- a/src/apps/testapps/testCellToCenterChild.c +++ b/src/apps/testapps/testCellToCenterChild.c @@ -38,8 +38,9 @@ SUITE(cellToCenterChild) { H3Index geoChild; t_assertSuccess( H3_EXPORT(latLngToCell)(¢roid, childRes, &geoChild)); - H3Index centerChild = - H3_EXPORT(cellToCenterChild)(h3Index, childRes); + H3Index centerChild; + t_assertSuccess(H3_EXPORT(cellToCenterChild)(h3Index, childRes, + ¢erChild)); t_assert( centerChild == geoChild, @@ -59,17 +60,23 @@ SUITE(cellToCenterChild) { TEST(sameRes) { int res = H3_EXPORT(getResolution)(baseHex); - t_assert(H3_EXPORT(cellToCenterChild)(baseHex, res) == baseHex, + H3Index child; + t_assertSuccess(H3_EXPORT(cellToCenterChild)(baseHex, res, &child)); + t_assert(child == baseHex, "center child at same resolution should return self"); } TEST(invalidInputs) { int res = H3_EXPORT(getResolution)(baseHex); - t_assert(H3_EXPORT(cellToCenterChild)(baseHex, res - 1) == 0, + H3Index child; + t_assert(H3_EXPORT(cellToCenterChild)(baseHex, res - 1, &child) == + E_RES_DOMAIN, "should fail at coarser resolution"); - t_assert(H3_EXPORT(cellToCenterChild)(baseHex, -1) == 0, - "should fail for negative resolution"); - t_assert(H3_EXPORT(cellToCenterChild)(baseHex, MAX_H3_RES + 1) == 0, + t_assert( + H3_EXPORT(cellToCenterChild)(baseHex, -1, &child) == E_RES_DOMAIN, + "should fail for negative resolution"); + t_assert(H3_EXPORT(cellToCenterChild)(baseHex, MAX_H3_RES + 1, + &child) == E_RES_DOMAIN, "should fail beyond finest resolution"); } } diff --git a/src/apps/testapps/testCompactCells.c b/src/apps/testapps/testCompactCells.c index 021cf98024..26acc34bfc 100644 --- a/src/apps/testapps/testCompactCells.c +++ b/src/apps/testapps/testCompactCells.c @@ -198,7 +198,8 @@ SUITE(compactCells) { t_assertSuccess(H3_EXPORT(cellToChildren)(h3, res + 1, children)); // duplicate one index - children[arrSize - 1] = H3_EXPORT(cellToCenterChild)(h3, res + 1); + t_assertSuccess( + H3_EXPORT(cellToCenterChild)(h3, res + 1, &children[arrSize - 1])); H3Index *output = calloc(arrSize, sizeof(H3Index)); diff --git a/src/h3lib/include/h3api.h.in b/src/h3lib/include/h3api.h.in index 4c8e7278b5..07312232c0 100644 --- a/src/h3lib/include/h3api.h.in +++ b/src/h3lib/include/h3api.h.in @@ -526,7 +526,8 @@ DECLSPEC H3Error H3_EXPORT(cellToChildren)(H3Index h, int childRes, */ /** @brief returns the center child of the given cell at the specified * resolution */ -DECLSPEC H3Index H3_EXPORT(cellToCenterChild)(H3Index h, int childRes); +DECLSPEC H3Error H3_EXPORT(cellToCenterChild)(H3Index h, int childRes, + H3Index *child); /** @} */ /** @defgroup compactCells compactCells diff --git a/src/h3lib/lib/h3Index.c b/src/h3lib/lib/h3Index.c index e7ae08c94e..f734920811 100644 --- a/src/h3lib/lib/h3Index.c +++ b/src/h3lib/lib/h3Index.c @@ -271,17 +271,16 @@ H3Index _zeroIndexDigits(H3Index h, int start, int end) { * * @param h H3Index to find center child of * @param childRes The resolution to switch to - * - * @return H3Index of the center child, or H3_NULL if you actually asked for a - * parent + * @param child H3Index of the center child + * @return 0 (E_SUCCESS) on success */ -H3Index H3_EXPORT(cellToCenterChild)(H3Index h, int childRes) { - if (!_hasChildAtRes(h, childRes)) return H3_NULL; +H3Error H3_EXPORT(cellToCenterChild)(H3Index h, int childRes, H3Index *child) { + if (!_hasChildAtRes(h, childRes)) return E_RES_DOMAIN; h = _zeroIndexDigits(h, H3_GET_RESOLUTION(h) + 1, childRes); H3_SET_RESOLUTION(h, childRes); - - return h; + *child = h; + return E_SUCCESS; } /** diff --git a/website/docs/core-library/usage.md b/website/docs/core-library/usage.md index 05570ad704..253a74d011 100644 --- a/website/docs/core-library/usage.md +++ b/website/docs/core-library/usage.md @@ -17,7 +17,10 @@ The file `h3api.h.in` is preprocessed into the file `h3api.h` as part of H3's bu ## API preconditions -The H3 API expects valid input. Behavior of the library may be undefined when given invalid input. Indexes should be validated with `h3IsValid` or `h3UnidirectionalEdgeIsValid` as appropriate. +The H3 API expects valid input. Behavior of the library may be undefined when given invalid input. Indexes should be validated with `isValidCell` or `isValidDirectedEdge` as appropriate. + +The library attempts to validate inputs and return useful error codes if input is invalid. Which inputs are validated, and how precisely they are +validated, may change between versions of the library. As a result the specific error code returned may change. ## Function renaming