Title: #5450: kcm: add support for kerberos tgt renewals
justin-stephenson commented:
"""
I've started discussion upstream in krb5 about making public
functions for
cred marshalling in krb5. This would allow SSSD to avoid reimplementing
several hundred lines of parser logic to produce a krb5_creds object - which
seems preferable to me.
In the interest of providing other feedback, though, I've reviewed the code as
it stands - noting as usual that I'm not reviewing talloc logic and deep
SSSD-isms. There are several persistent problems that come up frequently
enough that I think it's clearer to call out here rather than marking each
instance.
Style violations (So that we're all on the same page, I'm referring to SSSD
coding style:
https://sssd.io/docs/developers/coding_style.html )
* Weird whitespace - it's easy to spot these reading through the github viewer
because things don't line up. Coding style says "MUST: No tabs all
indentation 4 spaces." (Presumably this isn't true for Makefiles, which
are
required to be tabs; there are some problems there too.)
* line lengths - While it's not a "MUST", < 80 character lines are
"HIGHLY
RECOMMENDED".
* goto labels: I believe the use of "immediate" should be replaced with
"done"/"fail".
* Multi-line comments "MUST" have `/*` and `*/` on their own lines, as well
as
being "real sentences" and resembling "real paragraphs".
* Mid-function variable declaration - "MUST: Always declare variables at the
top of the function or block."
General clarity things:
* `ret = foo; return ret;`. There's no need for this; just `return ret`.
* `return foo; done: return foo;`. The double return is redundant, and
moreover the label isn't necessary at all; just return directly within the
function.
Further comments are inline.
Thanks very much for the review. I have addressed most of the in-line comments, but I
still need to fix the style issues you mention here in this specific comment.
I'll push to the PR again once done.
"""
See the full comment at