Conversation
msteinert
force-pushed
the
support-openpam
branch
2 times, most recently
from
4134a2f to
fe671b5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #31 +/- ##
==========================================
+ Coverage 82.31% 82.66% +0.35%
==========================================
Files 2 2
Lines 147 150 +3
==========================================
+ Hits 121 124 +3
Misses 23 23
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| #ifdef OPENPAM | ||
| int pam_start_confdir(const char *service_name, const char *user, const struct pam_conv *pam_conversation, | ||
| const char *confdir, pam_handle_t **pamh) | ||
| { | ||
| if (pamh != NULL) | ||
| *pamh = NULL; | ||
|
|
||
| return PAM_SYSTEM_ERR; | ||
| } | ||
| #else |
There was a problem hiding this comment.
Wow, that's weird that __attribute__((weak)); doesn't work there... Are you sure there's any need for this (e.g. it should just check it at runtime)?
There was a problem hiding this comment.
Checked again in freebsd, and the weak check definitely works in freebsd, so it's weird that it doesn't in mac...
There was a problem hiding this comment.
I think it's because the default toolchain on Mac is Clang. The implementation of the weak attribute doesn't appear to be identical... I didn't really dig into it to figure out why.
There was a problem hiding this comment.
Thinking about this a little more, using #ifdef __clang__ is probably worse. My original solution (#ifdef OPENPAM) isn't fully sufficient either, since it's possible somebody might use clang on a Linux-PAM system.
I think the only truly platform-independent way to solve this is with dlsym. #ifdef OPENPAM is probably good enough for now to get things building for people who reported issues.
There was a problem hiding this comment.
This is what a dlsym solution might look like: support-openpam...support-openpam-libdl
There was a problem hiding this comment.
Mh, I see, I think I tried to build this with clang in linux and it did work, however, well... We can go with the dlsym if there's no other reliable way indeed.
There was a problem hiding this comment.
The following error occurred while linking the test cases. Presumably the same issue would occur linking an application.
$ go build
$ go test
# github.com/msteinert/pam/v2.test
/opt/homebrew/Cellar/go/1.24.3/libexec/pkg/tool/darwin_arm64/link: running cc failed: exit status 1
/usr/bin/cc -arch arm64 -Wl,-S -Wl,-x -o $WORK/b001/pam.test -Qunused-arguments /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/go.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000000.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000001.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000002.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000003.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000004.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000005.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000006.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000007.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000008.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000009.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000010.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000011.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000012.o /var/folders/m5/f15ps3xd7lz2gbn361wzn07r0000gp/T/go-link-3900121331/000013.o -O2 -g -lpam -lresolv -O2 -g -framework CoreFoundation
Undefined symbols for architecture arm64:
"_pam_start_confdir", referenced from:
__cgo_ea745b8c258d_Cfunc_pam_start_confdir in 000002.o
_check_pam_start_confdir in 000003.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
FAIL github.com/msteinert/pam/v2 [build failed]
I don't particularly love the dlsym approach, I'm open to suggestions. It looks like Linux-PAM does have a version macro, but the minor version is set to 0 and hasn't changed in 20 years.
msteinert
force-pushed
the
support-openpam
branch
from
23cc2ea to
9420276
Compare
Tests should only set the finalizer to check if a transaction ended if the start call was successful.
2 participants
fixes: #24, #30