Paul B. Henson wrote:
On Sun, Jun 15, 2014 at 06:04:20AM -0700, Howard Chu wrote:
ldap_pvt_gettime() returns structured time. There is no reason to then call lutil_tm2time() to turn it into seconds, and then call slap_timestamp() which must turn seconds into structured time again for formatting. Personally I would just sprintf a timestamp here using the lutil_tm structure.
You mean you'd copy the the struct lutil_tm to a struct tm and call strftime, or you'd actually duplicate the functionality of strftime yourself with a raw sprintf? It seems if you have an API function to generate a timestamp (or one to format a string time), it's cleaner to actually use them, even if it requires swapping some types around, than to duplicate their functionality inline yourself. But if that's what you want I'll work on it.
I would have just used a raw sprintf. It's not like the provided APIs do anything special.
A time_t is still needed in other parts of the function for comparisons, so it seems lutil_tm2time would still need to be called to aquire it, even if the timestamp is made using strftime or sprintf using structured time? Or you'd have to call both ldap_pvt_gettime and time().
No, good point, I'd overlooked the other uses of "now" in the function. In that case we may just go ahead with the patch in its current form.