Summary: | ASTERISK-06157: [patch] new function ast_get_time_t() | ||
Reporter: | Luigi Rizzo (rizzo) | Labels: | |
Date Opened: | 2006-01-22 07:41:58.000-0600 | Date Closed: | 2008-01-15 16:47:22.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Core/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) patch-scanf | |
Description: | there are several instances, in the code, where we try to sscanf() from a string into a time_t argument, and assign a default value on error. The problem here is that the size of time_t is not specified, so the format could be %ld or %d in a platform dependent way, and replicating the workarounds (casts, intermediate variables, ugly PRI_* format specifiers, or simply ignored compiler warnings) all the times is pointless and error prone. This patch provides a new function to be used to fetch a time_t value from a string, assign a default value in case of error, and return success/or failure to the caller. This not only moves portability issues to a single place, but will also enable us, should we need it at a later time, to extend the allowed formats for time_t arguments to something more than integer numbers. See 'Additional info' below for more. ****** ADDITIONAL INFORMATION ****** While we put in this (and possibly other functions of the same kind), i think we should also try to address the following two issues: - the place where these API are declared. All in one header file (in which case possible candidates would be config.h or strings.h or maybe utils.h) or each in the "closer" header (e.g. ast_get_ip() is in acl.h, but I don't think it makes a lot of sense there); - the order of arguments. All of them will have the source string, the destination address, and possibly a default value (to be used in case of errors) and zero or more 'range' arguments, modifiers, etc. I used the "scanf order" i.e. put the string first and the destination second. ast_get_ip() uses a different order. I am totally neutral here. In any case, i think we should start merging this code as it really removes a lot of redundant code, and we can always change the header file and order of arguments when a decision has been made. Also note that an ast_get_int() would really be handy here, because there are many places where we replicate code for checking that a number is within a given range and supply default values etc. | ||
Comments: | By: Kevin P. Fleming (kpfleming) 2006-02-14 17:24:48.000-0600 Committed to trunk, although I moved the prototype to strings.h and the function to utils.c. Also added proper doxygen documentation for the new API call. By: Digium Subversion (svnbot) 2008-01-15 16:47:22.000-0600 Repository: asterisk Revision: 10105 U trunk/apps/app_sayunixtime.c U trunk/apps/app_voicemail.c U trunk/channels/chan_iax2.c U trunk/channels/chan_sip.c U trunk/funcs/func_strings.c U trunk/include/asterisk/strings.h U trunk/res/res_agi.c U trunk/utils.c ------------------------------------------------------------------------ r10105 | kpfleming | 2008-01-15 16:47:21 -0600 (Tue, 15 Jan 2008) | 2 lines add API function for parsing strings to time_t (issue ASTERISK-6157, with mods) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=10105 |