brechtsanders / xlsxio

XLSX I/O - C library for reading and writing .xlsx files
MIT License
420 stars 113 forks source link

using dll with MSVC causes a crash while calling a free() after calling xlsxioread_sheet_next_cell #73

Closed dazkalt closed 4 years ago

dazkalt commented 4 years ago

msvc 2019 using dll and*.a files as libs example from readme

xlsxioreader xlsxioread; if ((xlsxioread = xlsxioread_open("test.xlsx")) == NULL) { fprintf(stderr, "Error opening .xlsx file\n"); return 1; }

//read values from first sheet char value; xlsxioreadersheet sheet; const char sheetname = NULL; printf("Contents of first sheet:\n"); if ((sheet = xlsxioread_sheet_open(xlsxioread, sheetname, XLSXIOREAD_SKIP_EMPTY_ROWS)) != NULL) { //read all rows while (xlsxioread_sheet_next_row(sheet)) { //read all columns while ((value = xlsxioread_sheet_next_cell(sheet)) != NULL) { printf("%s\t", value); free(value); //BOOM-BANG!!! } printf("\n"); } xlsxioread_sheet_close(sheet); }

It seems like it normally runs without calling free() and shows no memory leaks, after finishing program execution

brechtsanders commented 4 years ago

Which test.xlsx file did you have this issue with? Do you know what the value was of value before free(value) crashed?

dazkalt commented 4 years ago

Thanks for the response! It can be any file, for an example i have created a new file in ms office with a single cell, which has the value "Test". value = "Test" before crashing(a call of the "free(value)" function, i have also tried to use instead "LocalFree(value)" - program crashes anyways)

brechtsanders commented 4 years ago

I see. LocalFree() should definitely not be used. free() should be used, but chances are MSVC is using a different free() than MinGW-w64 which the DLL was built with. So maybe I should just create a library-specific free function so the right free() can be called from the DLL.

dazkalt commented 4 years ago

Indeed! This is most probably the case of an issue, i should have guessed this all by myself, obviously 2 compilers are using different memory managers.

brechtsanders commented 4 years ago

xlsxioread_free() has been added in release 0.2.29

peno64 commented 2 years ago

@brechtsanders I had the same issue because I used your examples (example_xlsxio_read, ...) which still use free() instead of xlsxioread_free() xlsxioread_free() indeed fixes the problem. It would be a good idea to modify your examples and documentation also.

brechtsanders commented 2 years ago

@peno64 Your right, I fixed this in example_xlsxio_read.c in the latest commit.

peno64 commented 2 years ago

@brechtsanders Great. Note that also example example_xlsxio_read_advanced.c uses free and should be adapted.

brechtsanders commented 2 years ago

@peno64 example example_xlsxio_read_advanced.c uses free() to clean up something allocated earlier with strdup() in the same file, so this case should not be replaced with xlsxioread_free()