holgern / pyedflib

pyedflib is a python library to read/write EDF+/BDF+ files based on EDFlib.
http://pyedflib.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
209 stars 121 forks source link

Compiler warnings #226

Open DimitriPapadopoulos opened 1 year ago

DimitriPapadopoulos commented 1 year ago

While building on Ubuntu 22.04:

pyedflib/_extensions/c/edflib.c:39: warning: ignoring ‘#pragma warning ’ [-Wunknown-pragmas]
   39 | #pragma warning( disable : 4996 ) // ignore unsafe strncpy
      | 
pyedflib/_extensions/c/edflib.c:40: warning: ignoring ‘#pragma warning ’ [-Wunknown-pragmas]
   40 | #pragma warning( disable : 4244 ) // ignore precision loss
      | 
pyedflib/_extensions/c/edflib.c: In function ‘edflib_check_edf_file’:
pyedflib/_extensions/c/edflib.c:2629:35: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘__off64_t’ {aka ‘long int’} [-Wformat=]
 2629 |                 printf("filesize %d != %d*%d+%d ",ftello(inputfile),edfhdr->recordsize, edfhdr->datarecords, edfhdr->hdrsize);
      |                                  ~^
      |                                   |
      |                                   int
      |                                  %ld
pyedflib/_extensions/c/edflib.c:2629:44: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘long long int’ [-Wformat=]
 2629 |                 printf("filesize %d != %d*%d+%d ",ftello(inputfile),edfhdr->recordsize, edfhdr->datarecords, edfhdr->hdrsize);
      |                                           ~^                                            ~~~~~~~~~~~~~~~~~~~
      |                                            |                                                  |
      |                                            int                                                long long int
      |                                           %lld
/usr/lib/python3/dist-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
   17 | #warning "Using deprecated NumPy API, disable it with " \
      |  ^~~~~~~
pyedflib/_extensions/_pyedflib.c:9231:27: warning: ‘__pyx_f_8pyedflib_11_extensions_9_pyedflib__chars’ defined but not used [-Wunused-function]
 9231 | static __Pyx_memviewslice __pyx_f_8pyedflib_11_extensions_9_pyedflib__chars(PyObject *__pyx_v_s) {
      |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
DimitriPapadopoulos commented 1 year ago

I would be great to rely on a recent version of EDFlib. Not sure how to extract the patches that need to be re-applied onto a new version.

skjerns commented 1 year ago

I agree. In the optimal scenario, we rewrite the code base such that the original C code remains untouched and all additions/changes are in external files.

Unfortunately, I do not have the resources to this right now :-/ it's probably a bit of work but less than expected. I updated the C library once.

DimitriPapadopoulos commented 1 year ago

So, if I understand you correctly, the C files contain more additions than actual changes. Maybe I'll give it a try.

skjerns commented 1 year ago

Yes! I think there are very little actual changes in the .c . I think some error messages have been added to the header files.

Best would be to compare the C library version with a diffchecker with the corresponding version of the original. Then you'll see what has actually been changed and what not :)

skjerns commented 1 year ago

It's even less than I thought: https://www.diffchecker.com/iE3iuUXR/ . Some changes might even be consistent with newer edflib versions, as I've added some 1.21 changes to the code base without integrating the entire file.

Seeing that it's not actually much, we might just be able to replace the C file as is and apply patches with external files. However, I'm not confident enough in C to know how best to do this. I think there are more changes in the header file, but it's a small file.

DimitriPapadopoulos commented 1 year ago

Modifying the functionality of a C function does require to modify its source code. Perhaps I can write an additional C function that runs extra code before/after calling the original EDFlib function. However, the existing and new functionality might be inextricably interlinked, enough to prevent splitting it into two functions.