Prepare Heimdal for a litany of new compiler warnings. #1167
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is so we can use RETURNING in the SQLite3 credentials cache to simplify things.
We need to treat the ccache given via `KRB5CCNAME` and `-c` as special for the purposes of dealing with `krb5_cc_new_unique()`. Specifically we'll need the `krb5_context` to know that its `default_cc_name` came from `KRB5CCNAME` or a `-c` command-line option.
When KRB5CCNAME was set (or a default ccache was set via krb5_cc_set_default_name()) don't search any other collections in krb5_cc_cache_match() or krb5_cccol_last_change_time(). Otherwise one could `kinit -c $SOME_NON_DEFAULT_CCACHE ...` and have kinit look for and find a suitable ccache in some other collection. The point of searching all collections is that if we know they're all relevant in the current context, then we can save the user (or login apps) having to set and coordinate KRB5CCNAME, which would be a significant UI/UX improvement. But we can only do that when we know that a ccache [collection] was not named either via KRB5CCNAME nor more explicitly. Well, _if_ the ccache named via KRB5CCNAME or otherwise happens to be the same as the default ccache for that cache type, then perhaps we should still search the other cache types' default collections too, but that will require more work.
…efault ccache type (fix heimdal#1124)
Calling krb5_cc_new_unique() with a non-NULL `type` argument that is not the same as the type of the default ccache (when that ccache is set via `KRB5CCNAME` or `krb5_cc_set_default_name()`) is currently surprising: it will create a ccache in the _default_ collection for the requested type, but if the user set `KRB5CCNAME` to a non-default ccache then this will be clearly not desired. To fix this we'll: a) let `krb5_cc_new_unique()` extract a collection name from its `type` argument, if present b) let `krb5_cc_new_unique()` use its `hint` argument as a collection name for the given collection `type` c) add a new `gen_new_2()` method to krb5_cc_ops that takes a collection name argument d) change all in-tree uses of `krb5_cc_new_unique()` to pas a collection name where possible TODO: - add the new `gen_new_2()` method to all in-tree cache types - change all in-tree uses of krb5_cc_new_unique() as described above - write a test
This function should be used in-tree and in ccache plugins to eliminate duplicate code and make cache name parsing more consistent so that we can have `KRB5CCNAME=<TYPE>` and `KRB5CCNAME=<TYPE>:` work consistently for all cache types and denote "use the default collection name for <TYPE>. All cache types in-tree support collections nowadays, even FILE. External cache plugins may not and need not. To support collections a cache must support the `set_default()` method of `krb5_cc_ops` -- see `krb5_cc_support_switch()`.
- Use WAL mode - Disable sync writes - Revamp the SQL schema - Replace triggers with foreign keys - Revamp scc_remove_cred() - Add scc_retrieve_cred() - When there is no default ccache, make the next one written to the default (maybe this is not a good idea?)
…one is (WIP) I've noticed that `klist -l` doesn't show which ccache is the primary one. This is fundamentally because we don't have a method for getting the primary ccache. This commit prepares for adding that.
added 29 commits
June 21, 2023 00:13
This reverts commit f05752653b477f8801e0262ae5257da4946b2170.
Mostly for read-only iov or krb5_data.
All for read-only krb5_data or gss_buffer_desc.
Requires an internal rk_UNCONST because of annoying execvp type, but that's better than rk_UNCONST in all the call sites.
I assume this is used read-only by ASN1_MALLOC_ENCODE.
Note: the rk_WFLAGS in configure.ac appears to be dead code, overridden by the rk_WFLAGS in cf/roken-frag.m4. This confusing state of affairs should be improved.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes, mostly to sprinkle const or rk_UNCONST as appropriate, prepare Heimdal to pass the following compiler warnings, tested with gcc7.4 and gcc12.2:
This covers all of the warnings mentioned in #1140 (comment) except for -Wshadow, which is already used, for -Wunused-variable, which I forgot about, and for -Wmissing-field-initializers, which I also forgot about. (Annoyance with-Wmissing-field-initializers in another project may have contributed to my amnesia in that case.)
Caveats:
So for now I'm leaving this PR in draft status until the other PRs are merged and issues resolved.