Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler warnings #226

Open
DimitriPapadopoulos opened this issue Jun 24, 2023 · 6 comments
Open

Compiler warnings #226

DimitriPapadopoulos opened this issue Jun 24, 2023 · 6 comments

Comments

@DimitriPapadopoulos
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Collaborator

skjerns commented Jun 27, 2023

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
Copy link
Contributor Author

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

@skjerns
Copy link
Collaborator

skjerns commented Jun 27, 2023

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
Copy link
Collaborator

skjerns commented Jun 27, 2023

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants