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

W. Trevor King wking at tremily.us
Thu May 17 20:58:36 EDT 2012


On Thu, May 17, 2012 at 05:47:13AM -0400, W. Trevor King wrote:
> On Thu, May 17, 2012 at 12:38:16AM -0400, Greg Hudson wrote:
> > On 05/16/2012 09:56 PM, W. Trevor King wrote:
> > > I'm sticking with the recommended "use tabs" style, even when the
> > > surrounding code uses spaces.
> > 
> > Where did you find a recommendation to use tabs?  We should clean that up.
> 
> In the source repo at `krb5/doc/coding-style`.  This is probably a
> better place for the canonical recomendations, because anyone
> submitting patches will have the source, but not everyone will read
> the wiki ;).

On Thu, May 17, 2012 at 08:46:24PM -0400, Tom Yu wrote:
> "W. Trevor King" <wking at tremily.us> writes:
> > 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

The reason I didn't do that with this patch because I wanted the
t_trace tests to validate my {ptype} format.  Finer grained patches
would need multiple t_trace patches, etc.  I'll try to split things up
though.

> 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.

Will do.  I'll post a git fork once it's setup.  I was in the
email-patch zone due to the preferences of the last few projects I've
sent patches to ;).

> > 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 above response to Greg.  I suppose I should have CCed the list.

> * 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.

Good point; will fix.

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

Ghah!  I fixed a few of those before submitting the patch, but I
suppose I should have grepped the patch.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: OpenPGP digital signature
Url : http://mailman.mit.edu/pipermail/krbdev/attachments/20120517/6295e296/attachment.bin


More information about the krbdev mailing list