Summary:	ASTERISK-04025: [patch] convert app_md5 to functions and fix an ast_separate_app_args bug
Reporter:	Russell Bryant (russell)	Labels:
Date Opened:	2005-05-01 22:49:53	Date Closed:	2008-01-15 15:33:09.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) appargfix.txt ( 1) appargs.alternate.txt ( 2) md5functions.rev2.txt
Description:	This implements 'md5' and 'md5check' as functions instead of applications.  For now, I have just added warnings to app_md5 to note that those applications have been deprecated. ****** ADDITIONAL INFORMATION ****** disclaimer is on file
Comments:	By: Kevin P. Fleming (kpfleming) 2005-05-01 23:05:37 In the normal md5 function, there's no need to duplicate the incoming data, because the function does not modify it. The normal function should be called MD5 (IMO), because that's the standard used around the Internet for it. The other could be CHECK_MD5 or Check_MD5 (or even CheckMD5 <G>)... I guess Mark needs to make a policy decision on a naming standard for these functions so we can get them merged. By: Russell Bryant (russell) 2005-05-02 09:39:14 new patch -- without the unneeded string duplication.   md5 --> MD5  and  md5check --> CheckMD5 By: Russell Bryant (russell) 2005-05-02 11:01:38 I'll update this again to use your ast_copy_string after my exam this afternoon :) By: Olle Johansson (oej) 2005-05-02 11:08:25 Change "md5sum" to "MD5 checksum" in the help texts. No one that is not a programmer will ever understand "md5sum". By: Olle Johansson (oej) 2005-05-02 11:09:21 Also, make sure that the warning is only issued once, not to annoy people. Look at kpflemings implementation of sipgetheader as SIP_HEADER. By: Tilghman Lesher (tilghman) 2005-05-02 11:45:58 Small bug:  if your string contains a '|' character, the ast_separate_app_args() function called with the last argument set to '2' will in effect lose everything after the '|'.  You really should call ast_separate_app_args() with the last argument set to 1 (i.e. look for a single delimiter). Also, in regards to function capitalization, we should have functions be either all uppercase or all lowercase.  Mixed case leads to confusion and typos.  Perhaps functions should be changed to be case-insensitive, much like apps are already case-insensitive. By: Russell Bryant (russell) 2005-05-02 15:11:09 rev2 uploaded with fixes for all of your suggestions.  I haven't changed the function names.  I'll wait for an executive decision on the naming convention. ... as for what Corydon has pointed out, I believe this to be a bug in ast_separate_args. If the last argument is set to 1, argv will be set correctly, however, argc will be be off by one. The problem is that the loop in ast_separate args looks ahead, even when it should not.  It should not be setting array[x+1] when x == arraylen-1 I have posted a patch to fix this, though you guys will have to help me with finding the most efficient way to do it.  :) By: Kevin P. Fleming (kpfleming) 2005-05-02 22:19:59 Corydon76: I agree, we already compare app names case-insensitively, function names should be compared that way too. Drumkilla: wouldn't the proper fix for the separate_args function be to just change the condition in the for() loop to 'x < (arraylen-1)'? By: Russell Bryant (russell) 2005-05-03 01:11:53 Well, the reason I did it like that was to keep the return value for argc accurate. By: Russell Bryant (russell) 2005-05-03 17:43:42 I posted a patch with an alternate fix for ast_separate_app_args.  Take your pick, I guess.  :) By: Kevin P. Fleming (kpfleming) 2005-05-03 23:59:50 I corrected the ast_separate_app_args problem in a different way, please look over my changes to make sure I didn't do anything silly... it works fine here, testing with Background() and Read(). I also added doxygen docs, so it will be documented that the last parameter is supposed to be the number of arguments you will accept, not the number of delimiters to look for. The functions have been committed to CVS HEAD with some minor modifications, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:33:09.000-0600 Repository: asterisk Revision: 5567 U   trunk/apps/app_md5.c U   trunk/pbx.c U   trunk/utils.c ------------------------------------------------------------------------ r5567 | kpfleming | 2008-01-15 15:33:08 -0600 (Tue, 15 Jan 2008) | 2 lines add MD5 and CHECK_MD5 functions, deprecate MD5 and MD5Check apps (bug ASTERISK-4025) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5567