rlancaste / stellarsolver

GNU General Public License v3.0
94 stars 47 forks source link

qsort_r detection relies on implicit function declaration #125

Closed fweimer-rh closed 4 months ago

fweimer-rh commented 1 year ago

The qsort_r function comes with different prototypes on different systems. There are some tests around NEED_DECLARE_QSORT_R and NEED_SWAP_QSORT_R that try to work around that. However, they only work with compilers which accept implicit function declarations.

Without support for implicit function declarations in the compiler, I get a build failure:

In file included from /builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/util/bl-sort.c:8:
/builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/util/bl-sort.c: In function 'bl_sort_rec':
/builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/os-features.h:123:17: error: implicit declaration of function 'qsort_r'
  123 | #define QSORT_R qsort_r
      |                 ^~~~~~~
/builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/util/bl-sort.c:68:9: note: in expansion of macro 'QSORT_R'
   68 |         QSORT_R(NODE_DATA(node), node->N, list->datasize, &ft, qcompare);
      |         ^~~~~~~

If I add -D_GNU_SOURCE to the build flags, the build fails because of a conflicting prototype:

In file included from /builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/blind/engine.c:40:
/builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/os-features.h:113:6: error: conflicting types for 'qsort_r'; have 'void(void *, size_t,  size_t,  void *, int (*)(void *, const void *, const void *))' {aka 'void(void *, long unsigned int,  long unsigned int,  void *, int (*)(void *, const void *, const void *))'}
  113 | void qsort_r(void *base, size_t nmemb, size_t sz,
      |      ^~~~~~~
In file included from /builddir/build/BUILD/stellarsolver-2.4/stellarsolver/astrometry/blind/engine.c:13:
/usr/include/stdlib.h:854:13: note: previous declaration of 'qsort_r' with type 'void(void *, size_t,  size_t,  int (*)(const void *, const void *, void *), void *)' {aka 'void(void *, long unsigned int,  long unsigned int,  int (*)(const void *, const void *, void *), void *)'}
  854 | extern void qsort_r (void *__base, size_t __nmemb, size_t __size,
      |             ^~~~~~~

So there does not seem to be a good way to build this program with C99 compilers on glibc (or other non-BSD platforms).

mattiaverga commented 1 year ago

It seems that astrometry >= 0.82 has overcame the qsort_r usage? https://github.com/dstndstn/astrometry.net/blob/0.82/include/astrometry/os-features.h

fweimer-rh commented 1 year ago

Certainly possible. I only checked this repository here and didn't track the full upstream relationship.

rlancaste commented 1 year ago

It is certainly possible that they did fix this upstream, but the problem is that I had to make so many changes to astrometry to make it compile on Windows, to make it play nicer on macOS, and to make it function as an internal library that would not require lots of temporary files that I can't update to a higher astrometry.net version without doing a lot of work. I had to remove a lot of code and modify a lot of functions to support the use case of StellarSolver, but astrometry.net needs that code for other use cases that StellarSolver does not require. I would like to update the base of StellarSolver to the latest astrometry.net, but I will need a large chunk of time to do so.

rlancaste commented 7 months ago

Thanks to @timsurber for his contribution, and to @knro for letting me know about it. @fweimer-rh and @mattiaverga can you confirm that the fix rectified the issue for you?

mattiaverga commented 4 months ago

I'm not expert, but I don't see the qsort_r warning in build logs anymore, so... I suppose this is fixed.

rlancaste commented 4 months ago

Thanks!