On Wed, 2013-01-23 at 14:00 +0100, Tomas Babej wrote: > On 01/22/2013 07:39 PM, Dmitri Pal wrote: > > On 01/22/2013 10:57 AM, Simo Sorce wrote: > >> On Tue, 2013-01-22 at 15:50 +0100, Tomas Babej wrote: > >>> Here I bring the updated version of the patch. Please note, that I > >>> *added* a flag attribute to ipadb_ldap_attr_to_krb5_timestamp > >>> function, that controls whether the timestamp will be checked for > >>> overflow or not. The reasoning behind this is that some attributes > >>> will not be set to future dates, due to their inherent nature - such > >>> as krbLastSuccessfulAuth or krbLastAdminUnlock. > >>> > >>> These are all related to past dates, and it would make no sense to set > >>> them to future dates, even manually. Therefore I'd rather represent > >>> negative values in these attributes as past dates. They would have to > >>> be set manually anyway, because they would represent timestamps before > >>> the beginning of the unix epoch, however, I find this approach better > >>> than pushing them up to year 2038 in case such things happens. > >>> > >>> Any objections to this approach? > >>> > >> I am not sure I understand what is the point of giving this option to > >> callers. A) How does an API user know when to use one or the other > >> option. B) What good does it make to have the same date return different > >> results based on a flag ? > >> > >> What will happen later on when MIT will 'fix' the 2038 limit by changing > >> the meaning of negative timestamps ? Keep in mind that right now > >> negative timestamps are not really valid in the MIT code. > >> > >> Unless there is a 'use' for getting negative timestamps I think it is > >> only harmful to allow it and consumers would only be confused on whether > >> it should be used or not. > >> > >> So my first impression is that you are a bit overthinking here and we > >> should instead always force the same behavior for all callers and always > >> check and enforce endoftime dates. > >> > >> Simo. > >> > > +1 > Ok, the patch does not distinguish between 'past' and 'future' > timestamps anymore. > > Please respond if you see any issues.
I am sorry I haven't replied yet. I meant to test the patch this time before ACKing given how fiddly this time issue seem to be but haven't had found the time so far. Just want to say I do not see any problem in the last incarnation of the patch, so if someone beats me to testing you have my blessing for an ACK. Simo. -- Simo Sorce * Red Hat, Inc * New York _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
