Fix signature of git_libgit2_opts #30821
Conversation
Keno
added
system:powerpc
stdlib/LibGit2/src/LibGit2.jl
Outdated
| @@ -1003,7 +1003,7 @@ function set_ssl_cert_locations(cert_loc) | |||
| cert_dir = isdir(cert_loc) ? cert_loc : Cstring(C_NULL) | |||
| cert_file == C_NULL && cert_dir == C_NULL && return | |||
| @check ccall((:git_libgit2_opts, :libgit2), Cint, | |||
| (Cint, Cstring, Cstring), | |||
| (Cint, Cstring, Cstring...), | |||
There was a problem hiding this comment.
the ... is on the wrong argument here. it should be (Cint, Cstring...)
There was a problem hiding this comment.
Does it make a difference? The prototype at the llvm level doesn't match either way. This syntax is a bit odd in any case. C doesn't restrict the remaining args to be Cstring and for our own conversions we do kind of need to know for each argument what it was intended to be at the C level. I think we may have to redesign that part, but that's a separate discussion of course.
There was a problem hiding this comment.
The llvm prototype will be exactly correct if you declare it correctly (modulo any implementation bugs). It might not make much difference, but why make it almost correct when you could make it entirely correct.
On most platforms this doesn't make a difference, but the PowerPC ABI uses the signature to decide whether or not to allocate a parameter save area. Without this, the caller does not, but the callee assumes it's there causing the callee to overwrite critical parts of the caller stack. Fixes #27007
StefanKarpinski
added
triage
JeffBezanson
added
backport 1.0
and removed
triage
On most platforms this doesn't make a difference,
but the PowerPC ABI uses the signature to decide
whether or not to allocate a parameter save area.
Without this, the caller does not, but the callee
assumes it's there causing the callee to overwrite
critical parts of the caller stack.
Fixes #27007