70 refactor global mutable folder#85
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
===========================================
+ Coverage 58.41% 70.54% +12.13%
===========================================
Files 32 34 +2
Lines 2366 2397 +31
Branches 290 291 +1
===========================================
+ Hits 1382 1691 +309
+ Misses 977 698 -279
- Partials 7 8 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors libdedx’s error handling to use named error-code macros and updates file-access path resolution intended to remove dependence on a global mutable folder buffer.
Changes:
- Introduce
dedx_error.hwith centralizedDEDX_OK/DEDX_ERR_*error-code definitions and switch call sites away from raw integers. - Rework data-directory selection in
dedx_file_access.cto resolve the data path via a static cached helper and update file I/O functions accordingly. - Add
CONTRIBUTING.mddocumenting a C style convention (declare variables at top of block).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libdedx/dedx.h | Includes new public error-code header. |
| libdedx/dedx.c | Replaces numeric error codes with DEDX_ERR_* constants in core APIs. |
| libdedx/dedx_validate.c | Uses new error-code constants during config validation. |
| libdedx/dedx_tools.c | Uses new error-code constants for validation errors. |
| libdedx/dedx_periodic_table.h | Includes error-code header for shared constants. |
| libdedx/dedx_periodic_table.c | Switches to DEDX_OK / DEDX_ERR_* codes. |
| libdedx/dedx_mstar.c | Switches to DEDX_OK constant. |
| libdedx/dedx_file_access.h | Adds DEDX_PATH_SIZE for path buffers. |
| libdedx/dedx_file_access.c | Removes global mutable folder and updates path resolution + file I/O error codes. |
| libdedx/dedx_error.h | New public header defining error codes. |
| libdedx/CMakeLists.txt | Installs dedx_error.h as part of public headers. |
| CONTRIBUTING.md | Adds C coding-style guidance for variable declarations. |
Comments suppressed due to low confidence (4)
libdedx/dedx_file_access.c:82
- _dedx_convert_to_binary() calls
memset(&data, 0, _DEDX_MAXELEMENTS);, which only zeroes_DEDX_MAXELEMENTSbytes, not_DEDX_MAXELEMENTSfloats. This leaves most ofdata[]uninitialized and then copies it intocontainer.data; usesizeof(data)(or_DEDX_MAXELEMENTS * sizeof(float)) for the memset size.
length = 0;
i = 0;
memset(&data, 0, _DEDX_MAXELEMENTS);
temp = _dedx_split(line, ':', &length, 100);
temp[0][0] = ' ';
datalines = atoi(temp[2]);
while (i++ < datalines && !feof(fp)) {
if (fgets(line, 100, fp) == NULL) {
}
data[i - 1] = atof(line);
}
container.target = atoi(temp[0]);
container.ion = atoi(temp[1]);
container.length = datalines;
memcpy(&container.data, &data, _DEDX_MAXELEMENTS * sizeof(float));
fwrite(&container, sizeof(container), 1, out);
libdedx/dedx_file_access.c:87
- _dedx_convert_to_binary() allocates split tokens via _dedx_split() on every header line but only calls
free(temp)once at the end. This leaks the inner allocations (and leaks previoustempvalues when overwritten); use _dedx_free_split_temp(temp) each time you're done with a split result (and before overwritingtemp).
memset(&data, 0, _DEDX_MAXELEMENTS);
temp = _dedx_split(line, ':', &length, 100);
temp[0][0] = ' ';
datalines = atoi(temp[2]);
while (i++ < datalines && !feof(fp)) {
if (fgets(line, 100, fp) == NULL) {
}
data[i - 1] = atof(line);
}
container.target = atoi(temp[0]);
container.ion = atoi(temp[1]);
container.length = datalines;
memcpy(&container.data, &data, _DEDX_MAXELEMENTS * sizeof(float));
fwrite(&container, sizeof(container), 1, out);
}
}
free(temp);
fclose(fp);
fclose(out);
libdedx/dedx_file_access.c:333
- _dedx_get_i_value() never sets
*err = DEDX_OKon entry, so callers can see a stale error code even when the lookup succeeds (the function only sets *err on some failure paths). Initialize*errto DEDX_OK at the start for consistent error reporting.
float _dedx_get_i_value(int target, int state, int *err) {
const char *folder;
float pot;
char file[] = "compos.txt";
char path[DEDX_PATH_SIZE];
char str[100];
char **temp;
unsigned int items;
FILE *fp;
pot = 0.0;
items = 0;
folder = _dedx_get_data_path();
strcpy(path, folder);
strcat(path, file);
fp = fopen(path, "r");
if (fp == NULL) {
*err = DEDX_ERR_NO_COMPOS_FILE;
return 0;
}
while (!feof(fp)) {
if (fgets(str, 100, fp) == NULL) {
libdedx/dedx_file_access.c:433
- _dedx_get_atima_data() allocates
composbefore the search loop and then allocates it again when a match is found, leaking the first allocation. If no match is found it returns an allocated buffer with uninitialized contents and leaves *err as DEDX_OK. Allocate only once when a match is found (or initialize the buffer), and return NULL / set an appropriate error when the target is missing.
*err = DEDX_OK;
folder = _dedx_get_data_path();
strcpy(path, folder);
strcat(path, file);
fp = fopen(path, "r");
if (fp == NULL) {
*err = DEDX_ERR_NO_COMPOSITION;
return NULL;
}
compos = (float *) malloc(sizeof(float) * 4);
while (!feof(fp) && fgets(str, 100, fp) != NULL) {
temp = _dedx_split(str, '\t', &items, 100);
if (atoi(temp[0]) == target) {
compos = (float *) malloc(sizeof(float) * 4);
compos[0] = atof(temp[1]);
compos[1] = atof(temp[2]);
compos[2] = atof(temp[3]);
compos[3] = atof(temp[4]);
}
_dedx_free_split_temp(temp);
}
fclose(fp);
return compos;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…, dedx_periodic_table, and dedx_validate; add test cases for error codes
closes #70