[Home]

Summary:ASTERISK-05488: ast_localtime proposed change
Reporter:Luigi Rizzo (rizzo)Labels:
Date Opened:2005-11-07 04:51:57.000-0600Date Closed:2011-06-07 14:03:19
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) patch.ast_localtime
Description:another side effect of time_t being defined as different types (int, long, ...)
on different platforms, is that it becomes hard to pass arguments by reference
to a function such as
           ast_localtime(const time_t *timep, ...)
because unless the argument is already a time_t, we need to cast it explicitly,
and the cast is bound to fail because of different types on some platforms.

My suggestion here is that, since passing the time_t by reference gives us
absolutely no advantage (the pointer is the same size as the data, and
the argument is not modified by ast_localtime()), we should change the
function to accept the first argument by value instead. This way the
compiler will do automatically the appropriate conversion on each platform,
with significant increase in readability and robustness.

The attached patch implements the proposed change.

****** ADDITIONAL INFORMATION ******

Note that ast_localtime() is very rarely used. As you can see in the
attached patch, it is only used 6 times in the whole asterisk code,
plus a zillion times in say.c which is, however, just the result of
a gigantic copy&paste exercise, and will hopefully be subject to a
huge restructuring in the near future (at least, i have large
patches in the works which i am simply waiting to post until 1.2 is out).
Comments:By: Clod Patry (junky) 2005-11-07 07:08:38.000-0600

I think you forget to attach your patch here ;)

By: Luigi Rizzo (rizzo) 2005-11-07 07:27:36.000-0600

you are right, here it is...

By: Tilghman Lesher (tilghman) 2005-11-07 08:02:42.000-0600

As noted on the mailing list, you're first going to have to fix FreeBSD for the following sequence, which happens in nearly every software project:

gettimeofday(&tv);
localtime_r(&tv.tv_secs, &tm);

Now would be a good time to convert everything that uses number of seconds since the epoch to time_t in FreeBSD.