[PATCH] Convert DEBUG_REFERRALS to TRACE_* framework and add t_trace.c.

[Tom Yu]

"W. Trevor King" <wking at tremily.us> writes:

> Also:
> * Add comment to k5-trace.h documenting "{keytab}" format.
> * Add "{ptype}" format support in `krb5int_trace()'.
> * Add `principal_type_string()' for the "{ptype}" logic.
>
> ---
> On Tue, May 15, 2012 at 10:42:38PM -0400, W. Trevor King wrote:
>> On Tue, May 15, 2012 at 05:16:36PM -0400, Tom Yu wrote:
>> > The sort of diagnostics that you are looking for are probably better
>> > obtained through the KRB5_TRACE functionality.  I realize that we
>> > don't have trace points that provide that, but we could add them.
>> > ...
>> > Feel free to submit a patch that restores the debug function, but I
>> > think it would be even better if we replace the code that's under the
>> > DEBUG_REFERRALS conditional with trace points for KRB5_TRACE.  Would
>> > you be willing to help with that?  It would have the advantage of not
>> > requiring recompilation to get the diagnostics.
>> 
>> Here's a preliminary patch.  I'm trying to figure out how to get the
>> test suite running, but I haven't gotten there yet, so the patch is
>> mostly untested.  It does compile, though ;).
>
> I've got the test suite going now, and added t_trace.c to test the
> trace functionality in isolation from the Kerberos logic.  There are a
> few corners that I'm not sure how to reach (ENOMEM, etc.), but with
> this patch there is now 95% coverage of trace.c.

Thanks.  The patch looks reasonable to me on a quick skim.  It would
be easier for us to work with this patch if it were broken into a
separate patch for each logical unit, e.g.,

* Add missing {keytab} format documentation to k5-trace.h
* Add {ptype} format to krb5int_trace() (including principal_type_string)
* Add t_trace.c
* Convert DEBUG_REFERRALS to use KRB5_TRACE

Either making a series of commits on a GitHub (or other public Git
repository) fork (preferred) or sending a patch series in e-mail would
work.

> I'm sticking with the recommended "use tabs" style, even when the
> surrounding code uses spaces.  If local consistency is more important,
> I can submit another version (or sed them yourselves ;).

Which recommendation are you seeing to "use tabs"?  Our current
preference is for spaces, though there are exceptions.

(see http://k5wiki.kerberos.org/wiki/Coding_style/Formatting)

We could, of course, reformat the patches when applying them, but it
makes it easier if they are already in our preferred format.

Some minor things:

* TRACE_GET_CRED_VIA_TKT_EXT_RETURN is called with a ternary
  expression that turns an error code to a string; I believe that
  macro could use {kerr} instead, simplifying the call site.

* Several places use "principle" where "principal" should be.

Thanks again for helping work on this.