Summary: | ASTERISK-05488: ast_localtime proposed change | ||
Reporter: | Luigi Rizzo (rizzo) | Labels: | |
Date Opened: | 2005-11-07 04:51:57.000-0600 | Date Closed: | 2011-06-07 14:03:19 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | 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. |