[Bug 271234] devel/libgit2: qsort_r/qsort_s issues on 14-CURRENT

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271234

            Bug ID: 271234
           Summary: devel/libgit2: qsort_r/qsort_s issues on 14-CURRENT
           Product: Ports & Packages
           Version: Latest
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: Individual Port(s)
          Assignee: mfechner@FreeBSD.org
          Reporter: dim@FreeBSD.org
          Assignee: mfechner@FreeBSD.org
             Flags: maintainer-feedback?(mfechner@FreeBSD.org)

During an exp-run for llvm 16 (see bug 271047), it turned out that
devel/libgit2 failed to build with clang 16:

  /wrkdirs/usr/ports/devel/libgit2/work/libgit2-1.5.2/src/util/util.c:731:28:
error: incompatible function pointer types passing 'int (void *, const void *,
const void *)' to parameter of type 'int (*)(const void *, const void *, void
*)' [-Wincompatible-function-pointer-types]
          qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
                                    ^~~~~~~~~~~~~~~~~~~~~
  /usr/include/stdlib.h:397:11: note: passing argument to parameter here
      int (*)(const void *, const void *, void *), void *);
            ^

Clang is indeed right, as the version of qsort_s(3) in FreeBSD 13 and later has
the 'void *payload' parameter last:

  errno_t  qsort_s(void *, rsize_t, rsize_t,
      int (*)(const void *, const void *, void *), void *);

This could be fixed by putting the arguments in the right place for qsort_s(3),
but it turns out the rabbit hole goes a bit deeper: on 14-CURRENT, libgit2's
CMake configuration is not able to detect qsort_r(3), which is actually why it
chooses qsort_s():

  -- Checking prototype qsort_r for GIT_QSORT_R_BSD
  -- Checking prototype qsort_r for GIT_QSORT_R_BSD - False
  -- Checking prototype qsort_r for GIT_QSORT_R_GNU
  -- Checking prototype qsort_r for GIT_QSORT_R_GNU - False
  -- Looking for qsort_s
  -- Looking for qsort_s - found

The problem with the GIT_QSORT_R_BSD detection is due to the check in libgit2's
src/CMakeLists.txt, where it does:

  check_prototype_definition(qsort_r
          "void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int
(*compar)(void *, const void *, const void *))"
          "" "stdlib.h" GIT_QSORT_R_BSD)

and CMake attempts to define a function with a similar prototype in its test
program, which then fails to compile, at least on 14-CURRENT:

 
/wrkdirs/share/dim/ports/devel/libgit2/work/.build/CMakeFiles/CMakeScratch/TryCompile-tILE28/CheckPrototypeDefinition.c:14:6:
error: expected identifier or '('
  void qsort_r(void *base, size_t nmemb, size_t size, void *thunk, int
(*compar)(void *, const void *, const void *)) {
       ^
  /usr/include/stdlib.h:357:5: note: expanded from macro 'qsort_r'
      __generic(arg5, int (*)(void *, const void *, const void *),        \\
      ^
  /usr/include/sys/cdefs.h:323:2: note: expanded from macro '__generic'
          _Generic(expr, t: yes, default: no)
          ^

This is because in https://cgit.freebsd.org/src/commit/?id=af3c78886fd8d Ed
Schouten changed the prototype of qsort_r(3) to match POSIX, using a C11
_Generic macro. When CMake tries to compile its own custom definition of
qsort_r, that fails with the above compile error, because the macro gets
expanded in place of the function declaration.

So the summarized situation is:

* On 12.x and 13.x, qsort_r(3) is a plain function, and uses the old comparison
function type: 'int (*compar)(void *thunk, const void *elem1, const void
*elem2)'.
  Therefore, CMake detects GIT_QSORT_R_BSD, and libgit2's src/util/util.c uses
the correct comparison function type.
* On 14.x, qsort_r(3) is a macro, and uses the POSIX comparison function type:
'int (*compar)(const void *elem1, const void *elem2, void *thunk)'.
  Therefore, CMake fails to detect GIT_QSORT_R_BSD, and detects GIT_QSORT_S
instead, and libgit2's src/util/util.c uses an incorrect comparison function
type.

Possible fixes include:

* Adding __FreeBSD_version check, although base commit af3c78886fd8d did not
bump the version
* Adding additional CMake logic to correctly work around the _Generic macro
definition in 14.x's stdlib.h
* Fixing CMake's check_prototype_definition()
<https://cmake.org/cmake/help/latest/module/CheckPrototypeDefinition.html> to
put parentheses around the checked function names
* ... haven't thought or more possible fixes here, suggestions welcome

Preferably, any fix should be upstreamable.

-- 
You are receiving this mail because:
You are the assignee for the bug.