From: Uwe Hermann Date: Sun, 25 Dec 2011 18:59:15 +0000 (+0100) Subject: datastore.c: Improve error handling and docs. X-Git-Tag: libsigrok-0.1.0~216 X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=15278f3e9cf4c4a4a6c331e042f9935709343c82;p=libsigrok.git datastore.c: Improve error handling and docs. - Add Doxygen comments for all functions (some TODOs remain). - Check for invalid input parameters (such as NULL pointers etc). - Return SR_ERR_ARG upon invalid input parameters. - Make sr_datastore_put() return int instead of void, so we can pass an error code (SR_OK, SR_ERR_MALLOC, and so on) to the caller. --- diff --git a/datastore.c b/datastore.c index 774ec08c..fccb4d5b 100644 --- a/datastore.c +++ b/datastore.c @@ -26,13 +26,40 @@ static gpointer new_chunk(struct sr_datastore **ds); +/** + * Create a new datastore with the specified unit size. + * + * The unit size is fixed once the datastore is created, and cannot be + * changed later on, neither can data be added to the datastore with + * different unit sizes later. + * + * It is the caller's responsibility to free the allocated memory of the + * datastore via the sr_datastore_destroy() function, if no longer needed. + * + * TODO: Unitsize should probably be unsigned int or uint32_t or similar. + * TODO: This function should have a 'chunksize' parameter, and + * struct sr_datastore a 'chunksize' field. + * + * @param unitsize The unit size (>= 1) to be used for this datastore. + * @param ds Pointer to a variable which will hold the newly created + * datastore structure. + * + * @return SR_OK upon success, SR_ERR_MALLOC upon memory allocation errors, + * or SR_ERR_ARG upon invalid arguments. If something other than SR_OK + * is returned, the value of 'ds' is undefined. + */ int sr_datastore_new(int unitsize, struct sr_datastore **ds) { - if (!ds) - return SR_ERR; + if (!ds) { + sr_err("ds: %s: ds was NULL", __func__); + return SR_ERR_ARG; + } - if (unitsize <= 0) - return SR_ERR; /* TODO: Different error? */ + if (unitsize <= 0) { + sr_err("ds: %s: unitsize was %d, but it must be >= 1", + __func__, unitsize); + return SR_ERR_ARG; + } if (!(*ds = g_try_malloc(sizeof(struct sr_datastore)))) { sr_err("ds: %s: ds malloc failed", __func__); @@ -46,23 +73,64 @@ int sr_datastore_new(int unitsize, struct sr_datastore **ds) return SR_OK; } +/** + * Destroy the specified datastore and free the memory used by it. + * + * This will free the memory used by the data in the datastore's 'chunklist', + * by the chunklist data structure itself, and by the datastore struct. + * + * @param ds The datastore to destroy. + * + * @return SR_OK upon success, SR_ERR_ARG upon invalid arguments. + */ int sr_datastore_destroy(struct sr_datastore *ds) { GSList *chunk; - if (!ds) - return SR_ERR; + if (!ds) { + sr_err("ds: %s: ds was NULL", __func__); + return SR_ERR_ARG; + } for (chunk = ds->chunklist; chunk; chunk = chunk->next) g_free(chunk->data); g_slist_free(ds->chunklist); g_free(ds); + /* TODO: Set ds = NULL? */ + return SR_OK; } -void sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, - int in_unitsize, int *probelist) +/** + * Append some data to the specified datastore. + * + * TODO: More elaborate function description. + * + * TODO: This function should use the (not yet available) 'chunksize' field + * of struct sr_datastore (instead of hardcoding DATASTORE_CHUNKSIZE). + * TODO: in_unitsize and probelist are unused? + * TODO: A few of the parameters can be const. + * TODO: Handle new_chunk() returning NULL. + * TODO: Ideally, 'ds' should be unmodified upon errors. + * + * @param ds Pointer to the datastore which shall receive the data. + * Must not be NULL. + * @param data Pointer to the memory buffer containing the data to add. + * Must not be NULL. TODO: Data format? + * @param length Length of the data to add (in number of bytes). + * TODO: Should 0 be allowed as length? + * @param in_unitsize The unit size (>= 1) of the input data. + * @param probelist Pointer to a list of integers (probe numbers). The probe + * numbers in this list are 1-based, i.e. the first probe + * is expected to be numbered 1 (not 0!). Must not be NULL. + * + * @return SR_OK upon success, SR_ERR_MALLOC upon memory allocation errors, + * or SR_ERR_ARG upon invalid arguments. If something other than SR_OK + * is returned, the value/state of 'ds' is undefined. + */ +int sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, + int in_unitsize, int *probelist) { unsigned int stored; int capacity, size, num_chunks, chunk_bytes_free, chunk_offset; @@ -72,18 +140,49 @@ void sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, (void)in_unitsize; (void)probelist; + if (!ds) { + sr_err("ds: %s: ds was NULL", __func__); + return SR_ERR_ARG; + } + + /* Unitsize must not be 0, we'll divide by 0 otherwise. */ + if (ds->ds_unitsize == 0) { + sr_err("ds: %s: ds->ds_unitsize was 0", __func__); + return SR_ERR_ARG; + } + + if (!data) { + sr_err("ds: %s: data was NULL", __func__); + return SR_ERR_ARG; + } + + if (in_unitsize < 1) { + sr_err("ds: %s: in_unitsize was %d, but it must be >= 1", + __func__, in_unitsize); + return SR_ERR_ARG; + } + + if (!probelist) { + sr_err("ds: %s: probelist was NULL", __func__); + return SR_ERR_ARG; + } + + /* Get the last chunk in the list, or create a new one if needed. */ if (ds->chunklist == NULL) chunk = new_chunk(&ds); else chunk = g_slist_last(ds->chunklist)->data; + /* Get/calculate number of chunks, free space, etc. */ num_chunks = g_slist_length(ds->chunklist); capacity = (num_chunks * DATASTORE_CHUNKSIZE); chunk_bytes_free = capacity - (ds->ds_unitsize * ds->num_units); chunk_offset = capacity - (DATASTORE_CHUNKSIZE * (num_chunks - 1)) - chunk_bytes_free; + stored = 0; while (stored < length) { + /* No more free space left, allocate a new chunk. */ if (chunk_bytes_free == 0) { chunk = new_chunk(&ds); chunk_bytes_free = DATASTORE_CHUNKSIZE; @@ -100,17 +199,45 @@ void sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, chunk_bytes_free -= size; stored += size; } + ds->num_units += stored / ds->ds_unitsize; + + return SR_OK; } +/** + * Allocate a new memory chunk, append it to the datastore's chunklist. + * + * The newly allocated chunk is added to the datastore's chunklist by this + * function, and the return value additionally points to the new chunk. + * + * The allocated memory is guaranteed to be cleared. + * + * TODO: This function should use the datastore's 'chunksize' field instead + * of hardcoding DATASTORE_CHUNKSIZE. + * TODO: Return int, so we can return SR_OK / SR_ERR_ARG / SR_ERR_MALLOC? + * + * @param ds Pointer to a variable which holds the datastore structure. + * Must not be NULL. The contents of 'ds' are modified in-place. + * + * @return Pointer to the newly allocated chunk, or NULL upon failure. + */ static gpointer new_chunk(struct sr_datastore **ds) { gpointer chunk; - if (!(chunk = malloc(DATASTORE_CHUNKSIZE * (*ds)->ds_unitsize))) - return NULL; + if (!ds) { + sr_err("ds: %s: ds was NULL", __func__); + return NULL; /* TODO: SR_ERR_ARG later? */ + } + + chunk = g_try_malloc0(DATASTORE_CHUNKSIZE * (*ds)->ds_unitsize); + if (!chunk) { + sr_err("ds: %s: chunk malloc failed", __func__); + return NULL; /* TODO: SR_ERR_MALLOC later? */ + } (*ds)->chunklist = g_slist_append((*ds)->chunklist, chunk); - return chunk; + return chunk; /* TODO: SR_OK later? */ } diff --git a/sigrok-proto.h b/sigrok-proto.h index 2c3be8b1..2d2b5fd0 100644 --- a/sigrok-proto.h +++ b/sigrok-proto.h @@ -34,8 +34,8 @@ int sr_get_loglevel(void); int sr_datastore_new(int unitsize, struct sr_datastore **ds); int sr_datastore_destroy(struct sr_datastore *ds); -void sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, - int in_unitsize, int *probelist); +int sr_datastore_put(struct sr_datastore *ds, void *data, unsigned int length, + int in_unitsize, int *probelist); /*--- device.c --------------------------------------------------------------*/