Skip to content

Commit

Permalink
devel/libgit2: fix build with clang 16
Browse files Browse the repository at this point in the history
Clang 16 has a new error about incompatible function types, which shows
up when building devel/libgit2:

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

I submitted libgit2/libgit2#6555 and
libgit2/libgit2#6558 upstream to remedy the
situation and correctly detect all qsort variants, and these were merged
with a few minor changes.

This is an adjusted version of the upstream commits that applies on top
of libgit-1.5.2.

PR:		271234
Approved by:	mfechner (maintainer)
MFH:		2023Q2
  • Loading branch information
DimitryAndric committed May 14, 2023
1 parent 8340044 commit a62f1b5
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 0 deletions.
1 change: 1 addition & 0 deletions devel/libgit2/Makefile
Expand Up @@ -6,6 +6,7 @@
PORTNAME= libgit2
DISTVERSIONPREFIX= v
DISTVERSION= 1.5.2
PORTREVISION= 1
CATEGORIES= devel

MAINTAINER= mfechner@FreeBSD.org
Expand Down
145 changes: 145 additions & 0 deletions devel/libgit2/files/patch-github-pr6555
@@ -0,0 +1,145 @@
--- CMakeLists.txt.orig 2023-02-15 10:03:30 UTC
+++ CMakeLists.txt
@@ -96,7 +96,7 @@ include(CheckStructHasMember)
include(CheckFunctionExists)
include(CheckSymbolExists)
include(CheckStructHasMember)
-include(CheckPrototypeDefinition)
+include(CheckPrototypeDefinitionSafe)
include(AddCFlagIfSupported)
include(FindPkgLibraries)
include(FindThreads)
--- cmake/CheckPrototypeDefinitionSafe.cmake.orig 2023-05-14 12:22:20 UTC
+++ cmake/CheckPrototypeDefinitionSafe.cmake
@@ -0,0 +1,16 @@
+include(CheckPrototypeDefinition)
+
+function(check_prototype_definition_safe function prototype return header variable)
+ # temporarily save CMAKE_C_FLAGS and disable warnings about unused
+ # unused functions and parameters, otherwise they will always fail
+ # if ENABLE_WERROR is on
+ set(SAVED_CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+
+ disable_warnings(unused-function)
+ disable_warnings(unused-parameter)
+
+ check_prototype_definition("${function}" "${prototype}" "${return}" "${header}" "${variable}")
+
+ # restore CMAKE_C_FLAGS
+ set(CMAKE_C_FLAGS "${SAVED_CMAKE_C_FLAGS}")
+endfunction()
--- src/CMakeLists.txt.orig 2023-02-15 10:03:30 UTC
+++ src/CMakeLists.txt
@@ -58,15 +58,29 @@ add_feature_info(futimens GIT_USE_FUTIMENS "futimens s

# qsort

-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)
+# old-style FreeBSD qsort_r() has the 'context' parameter as the first argument
+# of the comparison function:
+check_prototype_definition_safe(qsort_r
+ "void (qsort_r)(void *base, size_t nmemb, size_t size, void *context, int (*compar)(void *, const void *, const void *))"
+ "" "stdlib.h" GIT_QSORT_BSD)

-check_prototype_definition(qsort_r
- "void qsort_r(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *arg)"
- "" "stdlib.h" GIT_QSORT_R_GNU)
+# GNU or POSIX qsort_r() has the 'context' parameter as the last argument of the
+# comparison function:
+check_prototype_definition_safe(qsort_r
+ "void (qsort_r)(void *base, size_t nmemb, size_t size, int (*compar)(const void *, const void *, void *), void *context)"
+ "" "stdlib.h" GIT_QSORT_GNU)

-check_function_exists(qsort_s GIT_QSORT_S)
+# C11 qsort_s() has the 'context' parameter as the last argument of the
+# comparison function, and returns an error status:
+check_prototype_definition_safe(qsort_s
+ "errno_t (qsort_s)(void *base, rsize_t nmemb, rsize_t size, int (*compar)(const void *, const void *, void *), void *context)"
+ "0" "stdlib.h" GIT_QSORT_C11)
+
+# MSC qsort_s() has the 'context' parameter as the first argument of the
+# comparison function, and as the last argument of qsort_s():
+check_prototype_definition_safe(qsort_s
+ "void (qsort_s)(void *base, size_t num, size_t width, int (*compare )(void *, const void *, const void *), void *context)"
+ "" "stdlib.h" GIT_QSORT_MSC)

# random / entropy data

--- src/features.h.in.orig 2023-02-15 10:03:30 UTC
+++ src/features.h.in
@@ -24,9 +24,10 @@
#cmakedefine GIT_REGEX_PCRE2
#cmakedefine GIT_REGEX_BUILTIN 1

-#cmakedefine GIT_QSORT_R_BSD
-#cmakedefine GIT_QSORT_R_GNU
-#cmakedefine GIT_QSORT_S
+#cmakedefine GIT_QSORT_BSD
+#cmakedefine GIT_QSORT_GNU
+#cmakedefine GIT_QSORT_C11
+#cmakedefine GIT_QSORT_MSC

#cmakedefine GIT_SSH 1
#cmakedefine GIT_SSH_MEMORY_CREDENTIALS 1
--- src/util/util.c.orig 2023-02-15 10:03:30 UTC
+++ src/util/util.c
@@ -18,7 +18,7 @@
# endif
# include <windows.h>

-# ifdef GIT_QSORT_S
+# ifdef GIT_QSORT_MSC
# include <search.h>
# endif
#endif
@@ -673,7 +673,7 @@ size_t git__unescape(char *str)
return (pos - str);
}

-#if defined(GIT_QSORT_S) || defined(GIT_QSORT_R_BSD)
+#if defined(GIT_QSORT_MSC) || defined(GIT_QSORT_BSD)
typedef struct {
git__sort_r_cmp cmp;
void *payload;
@@ -688,9 +688,11 @@ static int GIT_LIBGIT2_CALL git__qsort_r_glue_cmp(
#endif


-#if !defined(GIT_QSORT_R_BSD) && \
- !defined(GIT_QSORT_R_GNU) && \
- !defined(GIT_QSORT_S)
+#if !defined(GIT_QSORT_BSD) && \
+ !defined(GIT_QSORT_GNU) && \
+ !defined(GIT_QSORT_C11) && \
+ !defined(GIT_QSORT_MSC)
+
static void swap(uint8_t *a, uint8_t *b, size_t elsize)
{
char tmp[256];
@@ -716,17 +718,20 @@ static void insertsort(
for (j = i; j > base && cmp(j, j - elsize, payload) < 0; j -= elsize)
swap(j, j - elsize, elsize);
}
+
#endif

void git__qsort_r(
void *els, size_t nel, size_t elsize, git__sort_r_cmp cmp, void *payload)
{
-#if defined(GIT_QSORT_R_BSD)
+#if defined(GIT_QSORT_GNU)
+ qsort_r(els, nel, elsize, cmp, payload);
+#elif defined(GIT_QSORT_C11)
+ qsort_s(els, nel, elsize, cmp, payload);
+#elif defined(GIT_QSORT_BSD)
git__qsort_r_glue glue = { cmp, payload };
qsort_r(els, nel, elsize, &glue, git__qsort_r_glue_cmp);
-#elif defined(GIT_QSORT_R_GNU)
- qsort_r(els, nel, elsize, cmp, payload);
-#elif defined(GIT_QSORT_S)
+#elif defined(GIT_QSORT_MSC)
git__qsort_r_glue glue = { cmp, payload };
qsort_s(els, nel, elsize, git__qsort_r_glue_cmp, &glue);
#else

0 comments on commit a62f1b5

Please sign in to comment.