Various build enhancements #149
Conversation
This function has been in krb5 since 1999. Heimdal started using gssapi_krb5.h 2006, and this function predates that there too (2004). Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Not all uses of this function were previously guarded. It was added to MIT in 1.9 (2010), and to Heimdal in 2011. Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
(I originally planned to make the cred store extensions a hard requirement, but this turned out to be an easier fix than expected.) |
simo5
left a comment
simo5
left a comment
There was a problem hiding this comment.
Perhaps we should just remove the guard around the header file definitions, and just guard the function calls with ifdefs.
src/environ.c
Outdated
| for (int i = 0; i < arr->nelts; ++i) | ||
| apr_table_set(req->subprocess_env, elts[i].key, elts[i].val); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why this function moved around ?
| static void mag_set_ccname_envvar(request_rec *req, struct mag_config *cfg, | ||
| struct mag_conn *mc) | ||
| { | ||
| #ifdef HAVE_CRED_STORE |
There was a problem hiding this comment.
Why this guard here ?
No cred store function is called here, also won't a completely empty function cause compile errors on platforms w/o a cred store ?
There was a problem hiding this comment.
mag_set_ccname_envvar() previously wasn't called unless HAVE_CRED_STORE was true; see https://github.com/modauthgssapi/mod_auth_gssapi/blob/master/src/environ.c#L470-L474 . The reason is that mc->ccname only exists if HAVE_CRED_STORE, and otherwise it's not a member of mc.
It's a void function, so it shouldn't cause errors (and I checked complation without defining HAVE_CRED_STORE), but if there's a particular style you'd prefer for empty functions I can put it in an #else.
frozencemetery
force-pushed
the
cleanup_deps
branch
from
ceb0b71 to
59e5ee1
Compare
Refactor mag_set_ccname_envvar() logic slightly to hide contents. Add conditional logic to mag_create_dir_config(). Cleanup definition of HAVE_CRED_STORE in mod_auth_gssapi.h. Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
Fixed function movement. Let me know what you'd like done about the rest (don't know why github marked that as resolved, sorry). |
|
/lgtm |
2 participants
Make requirements on
gss_krb5_ccache_name()andgss_acquire_cred_with_password()hard. Also fix build when cred store extensions are not present (see #147).