diff --git a/remotefilelog/cdatapack/cdatapack.c b/remotefilelog/cdatapack/cdatapack.c index f80688631f..74f9a3331e 100644 --- a/remotefilelog/cdatapack/cdatapack.c +++ b/remotefilelog/cdatapack/cdatapack.c @@ -440,20 +440,24 @@ const uint8_t *getdeltachainlink( return ptr; } -delta_chain_t *getdeltachain( +delta_chain_t getdeltachain( const datapack_handle_t *handle, const uint8_t node[NODE_SZ]) { pack_chain_t *pack_chain = build_pack_chain(handle, node); if (pack_chain == NULL) { - return NULL; + // TODO: build_pack_chain needs to return a more detailed error code. + return (delta_chain_t) { GET_DELTA_CHAIN_NOT_FOUND }; } - delta_chain_t *delta_chain = malloc(sizeof(delta_chain_t)); - delta_chain->links_count = pack_chain->links_idx; - delta_chain->delta_chain_links = malloc( - delta_chain->links_count * sizeof(delta_chain_link_t)); - // TODO: error handling + delta_chain_t result; + result.links_count = pack_chain->links_idx; + result.delta_chain_links = malloc( + result.links_count * sizeof(delta_chain_link_t)); + if (result.delta_chain_links == NULL) { + result.code = GET_DELTA_CHAIN_OOM; + goto error_cleanup; + } for (int ix = 0; ix < pack_chain->links_idx; ix ++) { @@ -462,12 +466,13 @@ delta_chain_t *getdeltachain( const uint8_t *end = ptr + pack_chain->pack_chain_links[ix].data_sz; - delta_chain_link_t *link = &delta_chain->delta_chain_links[ix]; + delta_chain_link_t *link = &result.delta_chain_links[ix]; ptr = getdeltachainlink(ptr, link); if (ptr > end) { - abort(); + result.code = GET_DELTA_CHAIN_CORRUPT; + goto error_cleanup; } } @@ -480,19 +485,25 @@ delta_chain_t *getdeltachain( platform_madvise_away((void *) ptr, end - ptr); } + result.code = GET_DELTA_CHAIN_OK; + + goto cleanup; + +error_cleanup: + free(result.delta_chain_links); + +cleanup: + // free pack chain. - if (pack_chain != NULL) { - free(pack_chain->pack_chain_links); - free(pack_chain); - } + free(pack_chain->pack_chain_links); + free(pack_chain); - return delta_chain; + return result; } -void freedeltachain(delta_chain_t *chain) { - for (size_t ix = 0; ix < chain->links_count; ix ++) { - free((void *) chain->delta_chain_links[ix].delta); +void freedeltachain(delta_chain_t chain) { + for (size_t ix = 0; ix < chain.links_count; ix ++) { + free((void *) chain.delta_chain_links[ix].delta); } - free(chain->delta_chain_links); - free(chain); + free(chain.delta_chain_links); } diff --git a/remotefilelog/cdatapack/cdatapack.h b/remotefilelog/cdatapack/cdatapack.h index bde6426f3e..98ed37d26d 100644 --- a/remotefilelog/cdatapack/cdatapack.h +++ b/remotefilelog/cdatapack/cdatapack.h @@ -75,10 +75,18 @@ typedef struct _delta_chain_link_t { const uint8_t *delta; } delta_chain_link_t; +typedef enum { + GET_DELTA_CHAIN_OK, + GET_DELTA_CHAIN_OOM, + GET_DELTA_CHAIN_NOT_FOUND, + GET_DELTA_CHAIN_CORRUPT, +} get_delta_chain_code_t; + /** * This represents an entire delta chain. */ typedef struct _delta_chain_t { + get_delta_chain_code_t code; delta_chain_link_t *delta_chain_links; size_t links_count; } delta_chain_t; @@ -110,11 +118,11 @@ bool find( /** * Retrieves a delta chain for a given node. */ -extern delta_chain_t *getdeltachain( +extern delta_chain_t getdeltachain( const datapack_handle_t *handle, const uint8_t node[NODE_SZ]); -extern void freedeltachain(delta_chain_t *chain); +extern void freedeltachain(delta_chain_t chain); // this should really be private, but we need it for the cdatapack_dump tool. extern const uint8_t *getdeltachainlink( diff --git a/remotefilelog/cdatapack/cdatapack_get.c b/remotefilelog/cdatapack/cdatapack_get.c index 0589a7587d..ea5e44a600 100644 --- a/remotefilelog/cdatapack/cdatapack_get.c +++ b/remotefilelog/cdatapack/cdatapack_get.c @@ -41,7 +41,12 @@ int main(int argc, char *argv[]) { unhexlify(argv[2], NODE_SZ * 2, binhash); - delta_chain_t *chain = getdeltachain(handle, binhash); + delta_chain_t chain = getdeltachain(handle, binhash); + + if (chain.code != GET_DELTA_CHAIN_OK) { + fprintf(stderr, "error retrieving delta chain (code=%d)\n", chain.code); + return 1; + } const char *last_filename = NULL; uint16_t last_filename_sz = 0; @@ -52,8 +57,8 @@ int main(int argc, char *argv[]) { char deltabase_buffer[NODE_SZ * 2]; char sha_buffer[NODE_SZ * 2]; - for (int ix = 0; ix < chain->links_count; ix ++) { - delta_chain_link_t *link = &chain->delta_chain_links[ix]; + for (int ix = 0; ix < chain.links_count; ix ++) { + delta_chain_link_t *link = &chain.delta_chain_links[ix]; SHA_CTX ctx; SHA1_Init(&ctx); diff --git a/remotefilelog/cdatapack/py-cdatapack.c b/remotefilelog/cdatapack/py-cdatapack.c index f09c76f03f..dd9f2c55cd 100644 --- a/remotefilelog/cdatapack/py-cdatapack.c +++ b/remotefilelog/cdatapack/py-cdatapack.c @@ -198,19 +198,30 @@ static PyObject *cdatapack_getdeltachain( return NULL; } - delta_chain_t *chain = getdeltachain(self->handle, (const uint8_t *) node); - if (chain == NULL) { + delta_chain_t chain = getdeltachain(self->handle, (const uint8_t *) node); + if (chain.code == GET_DELTA_CHAIN_OOM) { + PyErr_NoMemory(); + return NULL; + } else if (chain.code == GET_DELTA_CHAIN_NOT_FOUND) { Py_RETURN_NONE; + } else if (chain.code != GET_DELTA_CHAIN_OK) { + // corrupt, etc. + PyErr_Format( + PyExc_ValueError, + "unknown error reading node %.*s", (int) node_sz, node); + return NULL; + } + PyObject *result = PyList_New(chain.links_count); + if (result == NULL) { + goto err_cleanup; } - PyObject *result = PyList_New(chain->links_count); - // TODO: error checking - for (int ix = 0; ix < chain->links_count; ix ++) { + for (int ix = 0; ix < chain.links_count; ix ++) { PyObject *tuple = NULL; PyObject *name = NULL, *retnode = NULL, *deltabasenode = NULL, *delta = NULL; - delta_chain_link_t *link = &chain->delta_chain_links[ix]; + delta_chain_link_t *link = &chain.delta_chain_links[ix]; name = PyString_FromStringAndSize(link->filename, link->filename_sz); retnode = PyString_FromStringAndSize((const char *) link->node, NODE_SZ);