stellarsolver icon indicating copy to clipboard operation
stellarsolver copied to clipboard

qsort_r detection relies on implicit function declaration

Open fweimer-rh opened this issue 2 years ago • 4 comments

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).

fweimer-rh avatar Jan 09 '23 12:01 fweimer-rh

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

mattiaverga avatar Jan 09 '23 17:01 mattiaverga

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

fweimer-rh avatar Jan 09 '23 18:01 fweimer-rh

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 avatar Jul 30 '23 14:07 rlancaste

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?

rlancaste avatar Apr 11 '24 02:04 rlancaste

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

mattiaverga avatar Jul 07 '24 07:07 mattiaverga

Thanks!

rlancaste avatar Jul 07 '24 09:07 rlancaste