Fine to go to me if you considered Dimitry's comments. Xuelei
On Aug 8, 2013, at 18:09, Weijun Wang <[email protected]> wrote: > Can I consider the fix reviewed and it's fine? > > Thanks > Max > > On 8/7/13 9:32 PM, Xuelei Fan wrote: >> On 8/7/2013 9:30 PM, Weijun Wang wrote: >>> First, thanks for your feedbacks. >>> >>> I only intended to fix etypes in this bug and since I don't have a lot >>> of experience on native kerberos on Mac (it is the Heimdal impl instead >>> of MIT's) I didn't want to touch a lot. >>> >>> Precisely, comparing only "krbtgt" is not enough. When doing cross-realm >>> auth from R1 to R2, it's likely to have "krbtgt/R2@R1" in ccache and it >>> should not used as initial TGT. >>> >>> Shall we fix this in another bug when I (or QE) are more familiar with >>> native krb5 on Mac? >> OK to me. >> >> Xuelei >> >>> Thanks >>> Max >>> >>> On 8/7/13 9:09 PM, Xuelei Fan wrote: >>>> On 8/7/2013 7:53 PM, Dmitry Samersoff wrote: >>>>> Xuelei, >>>>> >>>>> 1. strncmp calls strlen at first, so explicit call to strlen is not >>>>> necessary. >>>> I was wondering to make the comparing when the length of serverName is >>>> bigger than strlen("krbtgt"). For example, "krbtgt_extra". Mine >>>> suggested code is incorrect, as the output name of krb5_unparse_name may >>>> be "krbtgt_extra/h.o.s.t@realm", but not "krbtgt_extra". >>>> >>>> It's a little problem, but we might want to make the comparing more >>>> precisely. >>>> >>>>> 2. strlen("krbtgt") == sizeof("krbtgt")-1 >>>>> as sizeof count terminating 0. >>>> You are right. >>>> >>>> Xuelei >>>> >>>>> -Dmitry >>>>> >>>>> >>>>> On 2013-08-07 15:31, Xuelei Fan wrote: >>>>>> On 8/7/2013 6:58 PM, Weijun Wang wrote: >>>>>>> >>>>>>> >>>>>>> On 8/7/13 5:23 PM, Dmitry Samersoff wrote: >>>>>>>> Weijun, >>>>>>>> >>>>>>>> nativeccache.c: >>>>>>>> >>>>>>>> 322: Could you change strlen("krbtgt") to sizeof("krbtgt")-1 to >>>>>>>> save a >>>>>>>> bit of computer power? >>>>>>> >>>>>>> Sure. >>>>>> >>>>>> strncmp() is normally work with strlen() while comparing two >>>>>> strings, in >>>>>> case the length of the two string are not equal. >>>>>> >>>>>> - 322 if (strncmp (serverName, "krbtgt", strlen("krbtgt")) == 0 && >>>>>> + 322 if (strlen(serverName) == sizeof("krbtgt") && >>>>>> + strncmp (serverName, "krbtgt", sizeof("krbtgt")) == 0 && >>>>>> >>>>>> BTW, as it is a local function, would you like to add a "static" >>>>>> keyword >>>>>> to isIn() function? >>>>>> >>>>>> Xuelei >>
