[Freeipa-devel] [PATCHES] Fix getkeytab operation

Simo Sorce simo at redhat.com
Thu Nov 20 15:47:35 UTC 2014


On Thu, 20 Nov 2014 16:47:29 +0200
Alexander Bokovoy <abokovoy at redhat.com> wrote:

> On Thu, 20 Nov 2014, Nathaniel McCallum wrote:
> >On Thu, 2014-11-20 at 09:12 -0500, Simo Sorce wrote:
> >> On Thu, 20 Nov 2014 12:36:45 +0200
> >> Alexander Bokovoy <abokovoy at redhat.com> wrote:
> >>
> >> > On Wed, 19 Nov 2014, Simo Sorce wrote:
> >> > >----- Original Message -----
> >> > >> From: "Alexander Bokovoy" <abokovoy at redhat.com>
> >> > >[...]
> >> > >
> >> > >> Regarding the patchset itself:
> >> > >>
> >> > >> Patch 0001: fix 'wuld' in the commit message. The rest is
> >> > >> fine.
> >> > >
> >> > >Fixed.
> >> > >
> >> > >> Patch 0002:
> >> > >>  - ticket number is missing in the commit message
> >> > >
> >> > >Added.
> >> > >
> >> > >>  - perhaps, an instruction how to regenerate asn1 code can be
> >> > >> made a Makefile target? We don't need to call it ourselves
> >> > >> but this would simplify things in future
> >> > >
> >> > >Added make regenerate target to asn1c makefile
> >> > >
> >> > >>  - I'm little uncomfortable how ASN_DEBUG() output goes
> >> > >> explicitly to stderr but I guess this is something we
> >> > >> currently cannot override with DS-specific log printing, so
> >> > >> no big deal right now
> >> > >
> >> > >ASN_DEBUG() is currently disabled as EMIT_ASN_DEBUG is
> >> > >undefined, we can later provide a replacement ASN_DEBUG
> >> > >function to hook debugging, but given the same code is used in
> >> > >both DS plugins and ipa-getkeytab binary I did not want to
> >> > >assume anything, and how to wire it up (if we even want to)
> >> > >should probably be discussed at a later time.
> >> > >
> >> > >>  - any specific need to get asn1/compile committed? We don't
> >> > >> commit it in the client code (ipa-client/compile).
> >> > >
> >> > >Added 'compile' to .gitignore in second patch
> >> > >
> >> > >> Patch 0003: OK
> >> > >
> >> > >Nothing changed here.
> >> > >
> >> > >I also remembered the patch naming policy :-) so new patch
> >> > >names/numbers are 514,515,516, third revision.
> >> > Thanks. The only complaint I have left is number of whitespace
> >> > errors that git says are in the 515th patch.
> >>
> >> Yeah the autogenerated code is not a pretty sight style-wise, do we
> >> want to run an automatic indenter on it ?
> >> I was hesitant to do so, but I wouldn't mind adding that, if we
> >> feel strongly about it.
> >
> >Let's please not try to correct autogenerated code.
> I'm not tied to this but Simo now thinks it is better to run indenter
> in the generator rule as this will give less problems in actual
> comparison noise that git diff would give.
> 
> I'll make sure to talk back to asn1c author to see if we can improve
> its generators upstream.

So given Nathaniel doesn't like to touch autogenerated code I'll leave
it as it is.
I am going to push with the only change being to remove
asn1/config.h.in~ with was added to the second commit by mistake.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the Freeipa-devel mailing list