Phase 0 of #149: docs and guards, no behaviour change

include/dedx.h

@@ -110,8 +110,16 @@ enum {
 
 /** @} */
 
+/** @brief Minimum buffer size (in bytes, including the terminating NUL) that
+ *  dedx_get_error_code() requires from its @p err_str argument. The longest message
+ *  currently shipped is well under this; the margin is intentional headroom for
+ *  future messages.
+ */
+#define DEDX_ERROR_STRING_MAX 64
+
 /** @brief Translate a numeric error code to a human-readable string.
- *  @param[out] err_str  Buffer to receive the error description (caller-allocated).
+ *  @param[out] err_str  Buffer to receive the error description (caller-allocated,
+ *                       at least DEDX_ERROR_STRING_MAX bytes).
  *  @param[in]  err      Error code returned by a libdedx function.
  */
 void dedx_get_error_code(char *err_str, int err);
@@ -199,14 +207,14 @@ float dedx_get_density(int material, int *err);
  */
 int dedx_is_gas(int target, int *err);
 
-/** @brief Return a null-terminated list of supported program identifiers.
- *  @return Pointer to a static array terminated by 0; do not free.
+/** @brief Return a -1-terminated list of supported program identifiers.
+ *  @return Pointer to a static array terminated by -1; do not free.
  */
 const int *dedx_get_program_list(void);
 
-/** @brief Return a null-terminated list of materials supported by a program.
+/** @brief Return a -1-terminated list of materials supported by a program.
  *  @param[in] program  Program identifier.
- *  @return Pointer to a static array terminated by 0; do not free.
+ *  @return Pointer to a static array terminated by -1; do not free.
  */
 const int *dedx_get_material_list(int program);
 
@@ -248,9 +256,9 @@ const int *dedx_get_material_list(int program);
 void dedx_get_material_list_for_ion(
     int program, int ion, int *materials, unsigned int max_materials, unsigned int *materials_len, int *err);
 
-/** @brief Return a null-terminated list of ions supported by a program.
+/** @brief Return a -1-terminated list of ions supported by a program.
  *  @param[in] program  Program identifier.
- *  @return Pointer to a static array terminated by 0; do not free.
+ *  @return Pointer to a static array terminated by -1; do not free.
  */
 const int *dedx_get_ion_list(int program);
 

src/dedx.c

@@ -108,85 +108,55 @@ void dedx_free_workspace(dedx_workspace *workspace, int *err) {
 }
 
 /*Return an explanation to the error code*/
+typedef struct {
+    int code;
+    const char *message;
+} dedx_error_entry;
+
+/* One entry per DEDX_ERR_* code in dedx_error.h. Table-driven so that adding a
+ * message can never desync from adding an entry to a switch's case labels --
+ * the previous switch-based version silently fell through to the default
+ * "No such error code." message for DEDX_ERR_INCONSISTENT_COMPOUND because no
+ * case had been added for it. tests/test_error_codes.c walks every code
+ * defined in dedx_error.h and asserts it resolves to a real message here. */
+static const dedx_error_entry dedx_error_table[] = {
+    {DEDX_OK, "No error."},
+    {DEDX_ERR_NO_COMPOS_FILE, "Embedded density/I-value metadata is unavailable."},
+    {DEDX_ERR_NO_GAS_FILE, "Embedded gas-state metadata is unavailable."},
+    {DEDX_ERR_NO_CHARGE_FILE, "Embedded effective-charge metadata is unavailable."},
+    {DEDX_ERR_NO_BINARY_DATA, "Embedded stopping-power data is unavailable."},
+    {DEDX_ERR_NO_BINARY_ENERGY, "Embedded energy-grid data is unavailable."},
+    {DEDX_ERR_WRITE_FAILED, "Unable to write to disk."},
+    {DEDX_ERR_NO_ENERGY_FILE, "Legacy energy source data is unavailable."},
+    {DEDX_ERR_NO_DATA_FILE, "Legacy stopping-power source data is unavailable."},
+    {DEDX_ERR_NO_NAMES_FILE, "Reserved internal metadata code."},
+    {DEDX_ERR_NO_COMPOSITION, "Embedded elemental composition metadata is unavailable."},
+    {DEDX_ERR_ENERGY_OUT_OF_RANGE, "Energy out of bounds."},
+    {DEDX_ERR_TARGET_NOT_FOUND, "Target is not in the embedded metadata."},
+    {DEDX_ERR_COMBINATION_NOT_FOUND, "Target and ion combination is not in the embedded data."},
+    {DEDX_ERR_INVALID_DATASET_ID, "ID does not exist."},
+    {DEDX_ERR_NOT_AN_ELEMENT, "Target is not an atomic element."},
+    {DEDX_ERR_ESTAR_NOT_IMPL, "ESTAR is not implemented yet."},
+    {DEDX_ERR_ION_NOT_SUPPORTED_MSTAR, "Ion is not supported for MSTAR."},
+    {DEDX_ERR_ION_NOT_SUPPORTED, "Ion is not supported for requested table."},
+    {DEDX_ERR_RHO_REQUIRED, "Rho must be specified in this configuration."},
+    {DEDX_ERR_ION_A_REQUIRED, "ion_a must be specified in this configuration."},
+    {DEDX_ERR_INVALID_I_VALUE, "I value must be larger than zero."},
+    {DEDX_ERR_INCONSISTENT_COMPOUND, "Compound specification is inconsistent."},
+    {DEDX_ERR_INVALID_INTERPOLATION_MODE, "Interpolation mode is not supported."},
+    {DEDX_ERR_NO_MEMORY, "Out of memory"},
+};
+
 void dedx_get_error_code(char *err_str, int err) {
-    switch (err) {
-    case DEDX_OK:
-        strcpy(err_str, "No error.");
-        break;
-    case DEDX_ERR_NO_COMPOS_FILE:
-        strcpy(err_str, "Embedded density/I-value metadata is unavailable.");
-        break;
-    case DEDX_ERR_NO_GAS_FILE:
-        strcpy(err_str, "Embedded gas-state metadata is unavailable.");
-        break;
-    case DEDX_ERR_NO_CHARGE_FILE:
-        strcpy(err_str, "Embedded effective-charge metadata is unavailable.");
-        break;
-    case DEDX_ERR_NO_BINARY_DATA:
-        strcpy(err_str, "Embedded stopping-power data is unavailable.");
-        break;
-    case DEDX_ERR_NO_BINARY_ENERGY:
-        strcpy(err_str, "Embedded energy-grid data is unavailable.");
-        break;
-    case DEDX_ERR_WRITE_FAILED:
-        strcpy(err_str, "Unable to write to disk.");
-        break;
-    case DEDX_ERR_NO_ENERGY_FILE:
-        strcpy(err_str, "Legacy energy source data is unavailable.");
-        break;
-    case DEDX_ERR_NO_DATA_FILE:
-        strcpy(err_str, "Legacy stopping-power source data is unavailable.");
-        break;
-    case DEDX_ERR_NO_NAMES_FILE:
-        strcpy(err_str, "Reserved internal metadata code.");
-        break;
-    case DEDX_ERR_NO_COMPOSITION:
-        strcpy(err_str, "Embedded elemental composition metadata is unavailable.");
-        break;
-    case DEDX_ERR_ENERGY_OUT_OF_RANGE:
-        strcpy(err_str, "Energy out of bounds.");
-        break;
-    case DEDX_ERR_TARGET_NOT_FOUND:
-        strcpy(err_str, "Target is not in the embedded metadata.");
-        break;
-    case DEDX_ERR_COMBINATION_NOT_FOUND:
-        strcpy(err_str, "Target and ion combination is not in the embedded data.");
-        break;
-    case DEDX_ERR_INVALID_DATASET_ID:
-        strcpy(err_str, "ID does not exist.");
-        break;
-    case DEDX_ERR_NOT_AN_ELEMENT:
-        strcpy(err_str, "Target is not an atomic element.");
-        break;
-    case DEDX_ERR_ESTAR_NOT_IMPL:
-        strcpy(err_str, "ESTAR is not implemented yet.");
-        break;
-    case DEDX_ERR_ION_NOT_SUPPORTED_MSTAR:
-        strcpy(err_str, "Ion is not supported for MSTAR.");
-        break;
-    case DEDX_ERR_ION_NOT_SUPPORTED:
-        strcpy(err_str, "Ion is not supported for requested table.");
-        break;
-    case DEDX_ERR_RHO_REQUIRED:
-        strcpy(err_str, "Rho must be specified in this configuration.");
-        break;
-    case DEDX_ERR_ION_A_REQUIRED:
-        strcpy(err_str, "ion_a must be specified in this configuration.");
-        break;
-    case DEDX_ERR_INVALID_I_VALUE:
-        strcpy(err_str, "I value must be larger than zero.");
-        break;
-    case DEDX_ERR_INVALID_INTERPOLATION_MODE:
-        strcpy(err_str, "Interpolation mode is not supported.");
-        break;
-    case DEDX_ERR_NO_MEMORY:
-        strcpy(err_str, "Out of memory");
-        break;
+    size_t i;
 
-    default:
-        strcpy(err_str, "No such error code.");
-        break;
+    for (i = 0; i < sizeof(dedx_error_table) / sizeof(dedx_error_table[0]); i++) {
+        if (dedx_error_table[i].code == err) {
+            strcpy(err_str, dedx_error_table[i].message);
+            return;
+        }
     }
+    strcpy(err_str, "No such error code.");
 }
 
 const char *dedx_get_program_name(int program) {
@@ -228,8 +198,8 @@ float dedx_get_i_value(int target, int *err) {
 #define DEDX_MAX_ELEMENT_Z 112
 
 int dedx_get_nucleon_number(int ion, int *err) {
-    /* The internal lookup only guards the upper bound; reject ion <= 0 here
-     * so we never index the table with a negative offset. */
+    /* dedx_internal_get_nucleon() now also guards ion <= 0, but keep this explicit
+     * check here too so the public wrapper's contract does not rely on that detail. */
     if (ion < 1 || ion > DEDX_MAX_ELEMENT_Z) {
         *err = DEDX_ERR_NOT_AN_ELEMENT;
         return -1;
@@ -884,6 +854,10 @@ static int load_bethe_2(stopping_data *data, dedx_config *config, float *energy,
 
     dedx_internal_bethe_workspace *bethe =
         (dedx_internal_bethe_workspace *) calloc(1, sizeof(dedx_internal_bethe_workspace));
+    if (bethe == NULL) { /* LCOV_EXCL_START -- OOM guard, not exercised by tests */
+        *err = DEDX_ERR_NO_MEMORY;
+        return -1;
+    } /* LCOV_EXCL_STOP */
     for (i = 0; i < data->length; i++) {
         data->data[i] = dedx_internal_calculate_bethe_energy(bethe, energy[i], PZ, PA, TZ, TA, rho, pot);
     }

src/dedx_periodic_table.c

@@ -1,26 +1,29 @@
 #include "dedx_periodic_table.h"
 
+/* dedx_amu[]/dedx_nucl[] are indexed by id - 1, so id must also be bounded below:
+ * a caller-supplied id <= 0 (e.g. an unchecked custom-compound elements_id[] entry)
+ * would otherwise index the tables out of bounds. */
 float dedx_internal_get_atom_charge(int id, int *err) {
     *err = DEDX_OK;
-    if (id < 113) {
+    if (id >= 1 && id < 113) {
         return id;
     }
-    *err = DEDX_ERR_NOT_AN_ELEMENT; /* LCOV_EXCL_LINE */
-    return -1;                      /* LCOV_EXCL_LINE */
+    *err = DEDX_ERR_NOT_AN_ELEMENT;
+    return -1;
 }
 
 float dedx_internal_get_atom_mass(int id, int *err) {
     *err = DEDX_OK;
-    if (id < 113)
+    if (id >= 1 && id < 113)
         return dedx_amu[id - 1];
-    *err = DEDX_ERR_NOT_AN_ELEMENT; /* LCOV_EXCL_LINE */
-    return -1;                      /* LCOV_EXCL_LINE */
+    *err = DEDX_ERR_NOT_AN_ELEMENT;
+    return -1;
 }
 
 int dedx_internal_get_nucleon(int id, int *err) {
     *err = DEDX_OK;
-    if (id < 113)
+    if (id >= 1 && id < 113)
         return dedx_nucl[id - 1];
-    *err = DEDX_ERR_NOT_AN_ELEMENT; /* LCOV_EXCL_LINE */
-    return -1;                      /* LCOV_EXCL_LINE */
+    *err = DEDX_ERR_NOT_AN_ELEMENT;
+    return -1;
 }

src/dedx_validate.c

@@ -223,6 +223,17 @@ int dedx_internal_validate_config(dedx_config *config, int *err) {
         if (*err != 0)
             return -1;
     }
+
+    /* load_compound() sums weight[i] * compound_data[i].data[j] over
+     * elements_mass_fraction whenever elements_id is set, regardless of program or
+     * target. Neither block above runs for a tabulated program (program < 100) with
+     * a non-zero target, so a caller-supplied elements_id without elements_mass_fraction
+     * (and nothing to derive it from) would otherwise reach that sum with a NULL
+     * weight array. Reject it here instead of dereferencing NULL later. */
+    if (config->elements_id != NULL && config->elements_mass_fraction == NULL) {
+        *err = DEDX_ERR_INCONSISTENT_COMPOUND;
+        return -1;
+    }
     return 0;
 }
 

tests/test_error_codes.c

@@ -2,6 +2,7 @@
 #include <dedx_tools.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 static int check_err(int got, int expected, const char *label) {
     if (got != expected) {
@@ -11,6 +12,67 @@ static int check_err(int got, int expected, const char *label) {
     return 0;
 }
 
+/* dedx_get_error_code() must resolve every DEDX_ERR_* code to a real message, not
+ * silently fall back to "No such error code." -- that is exactly how
+ * DEDX_ERR_INCONSISTENT_COMPOUND (211) went unnoticed before. There is no way to
+ * enumerate C preprocessor macros at runtime, so this list has to be kept in sync
+ * with dedx_error.h by hand; that is the point -- adding a new DEDX_ERR_* code
+ * without extending this test (and dedx_get_error_code()'s table) now fails CI
+ * instead of failing silently in the field. */
+static int check_has_message(int code, const char *label) {
+    char err_str[DEDX_ERROR_STRING_MAX];
+
+    dedx_get_error_code(err_str, code);
+    if (strcmp(err_str, "No such error code.") == 0) {
+        fprintf(stderr, "FAIL %s: dedx_get_error_code(%d) has no message\n", label, code);
+        return 1;
+    }
+    return 0;
+}
+
+static int test_error_code_strings_complete(void) {
+    int failures = 0;
+
+    failures += check_has_message(DEDX_OK, "DEDX_OK");
+    failures += check_has_message(DEDX_ERR_NO_COMPOS_FILE, "DEDX_ERR_NO_COMPOS_FILE");
+    failures += check_has_message(DEDX_ERR_NO_GAS_FILE, "DEDX_ERR_NO_GAS_FILE");
+    failures += check_has_message(DEDX_ERR_NO_CHARGE_FILE, "DEDX_ERR_NO_CHARGE_FILE");
+    failures += check_has_message(DEDX_ERR_NO_BINARY_DATA, "DEDX_ERR_NO_BINARY_DATA");
+    failures += check_has_message(DEDX_ERR_NO_BINARY_ENERGY, "DEDX_ERR_NO_BINARY_ENERGY");
+    failures += check_has_message(DEDX_ERR_WRITE_FAILED, "DEDX_ERR_WRITE_FAILED");
+    failures += check_has_message(DEDX_ERR_NO_ENERGY_FILE, "DEDX_ERR_NO_ENERGY_FILE");
+    failures += check_has_message(DEDX_ERR_NO_DATA_FILE, "DEDX_ERR_NO_DATA_FILE");
+    failures += check_has_message(DEDX_ERR_NO_NAMES_FILE, "DEDX_ERR_NO_NAMES_FILE");
+    failures += check_has_message(DEDX_ERR_NO_COMPOSITION, "DEDX_ERR_NO_COMPOSITION");
+    failures += check_has_message(DEDX_ERR_ENERGY_OUT_OF_RANGE, "DEDX_ERR_ENERGY_OUT_OF_RANGE");
+    failures += check_has_message(DEDX_ERR_TARGET_NOT_FOUND, "DEDX_ERR_TARGET_NOT_FOUND");
+    failures += check_has_message(DEDX_ERR_COMBINATION_NOT_FOUND, "DEDX_ERR_COMBINATION_NOT_FOUND");
+    failures += check_has_message(DEDX_ERR_INVALID_DATASET_ID, "DEDX_ERR_INVALID_DATASET_ID");
+    failures += check_has_message(DEDX_ERR_NOT_AN_ELEMENT, "DEDX_ERR_NOT_AN_ELEMENT");
+    failures += check_has_message(DEDX_ERR_ESTAR_NOT_IMPL, "DEDX_ERR_ESTAR_NOT_IMPL");
+    failures += check_has_message(DEDX_ERR_ION_NOT_SUPPORTED_MSTAR, "DEDX_ERR_ION_NOT_SUPPORTED_MSTAR");
+    failures += check_has_message(DEDX_ERR_ION_NOT_SUPPORTED, "DEDX_ERR_ION_NOT_SUPPORTED");
+    failures += check_has_message(DEDX_ERR_RHO_REQUIRED, "DEDX_ERR_RHO_REQUIRED");
+    failures += check_has_message(DEDX_ERR_ION_A_REQUIRED, "DEDX_ERR_ION_A_REQUIRED");
+    failures += check_has_message(DEDX_ERR_INVALID_I_VALUE, "DEDX_ERR_INVALID_I_VALUE");
+    failures += check_has_message(DEDX_ERR_INCONSISTENT_COMPOUND, "DEDX_ERR_INCONSISTENT_COMPOUND");
+    failures += check_has_message(DEDX_ERR_INVALID_INTERPOLATION_MODE, "DEDX_ERR_INVALID_INTERPOLATION_MODE");
+    failures += check_has_message(DEDX_ERR_NO_MEMORY, "DEDX_ERR_NO_MEMORY");
+
+    /* An unrecognised code must still fall back to the default message rather than,
+     * say, leaving the buffer untouched or crashing. */
+    {
+        char err_str[DEDX_ERROR_STRING_MAX];
+        dedx_get_error_code(err_str, 99999);
+        if (strcmp(err_str, "No such error code.") != 0) {
+            fprintf(stderr, "FAIL unknown error code should report 'No such error code.', got \"%s\"\n", err_str);
+            failures++;
+        }
+    }
+
+    return failures;
+}
+
 int main(void) {
     int failures = 0;
     int err = 0;
@@ -167,5 +229,31 @@ int main(void) {
     dedx_free_config(cfg, &err);
     dedx_free_workspace(ws, &err);
 
+    /* Regression test for a NULL dereference: a tabulated program (< 100) with a
+     * real target and a caller-supplied elements_id but no elements_mass_fraction
+     * (and no elements_atoms to derive it from) used to sail through
+     * dedx_internal_validate_config() untouched -- neither the program >= 100 branch
+     * nor the target == 0 branch runs for this combination -- and then crash inside
+     * load_compound(), which sums weight[i] * compound_data[i].data[j] with
+     * weight == NULL. It must now fail cleanly instead. */
+    ws = dedx_allocate_workspace(1, &err);
+    cfg = calloc(1, sizeof(dedx_config));
+    cfg->program = DEDX_PSTAR;
+    cfg->ion = DEDX_PROTON;
+    cfg->target = DEDX_WATER;
+    cfg->elements_length = 2;
+    cfg->elements_id = calloc(2, sizeof(int));
+    cfg->elements_id[0] = DEDX_HYDROGEN;
+    cfg->elements_id[1] = DEDX_OXYGEN;
+    /* elements_atoms and elements_mass_fraction intentionally left NULL. */
+    err = 0;
+    dedx_load_config(ws, cfg, &err);
+    failures += check_err(err, DEDX_ERR_INCONSISTENT_COMPOUND, "elements_id without weights must be rejected");
+    failures += check_err(cfg->loaded, 0, "failed load should not mark config loaded");
+    dedx_free_config(cfg, &err);
+    dedx_free_workspace(ws, &err);
+
+    failures += test_error_code_strings_complete();
+
     return failures;
 }