Summary:	ASTERISK-07937: Check return codes everywhere
Reporter:	Markus Elfring (elfring)	Labels:
Date Opened:	2006-10-17 04:00:54	Date Closed:	2006-11-28 10:31:48.000-0600
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	I have seen in your source files (http://ftp.digium.com/pub/asterisk/releases/asterisk-1.4.0-beta2.tar.gz) that some return values are ignored. Would you like to add any checks for them? Examples: 1. manager.c: "complete_show_mancmd" http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutex_lock.html 2. muted.c: "add_channel" http://www.opengroup.org/onlinepubs/009695399/functions/strdup.html 3. ael_main.c: "create_name" http://www.opengroup.org/onlinepubs/009695399/functions/calloc.html Would you like to try the tool "http://splint.org/manual/html/sec8.html" to detect such open issues earlier?
Comments:	By: Russell Bryant (russell) 2006-10-17 11:20:07 This is a type of issue that would need to be handled on a case by case basis most likely.  Before working on any patches, if you aren't sure whether something needs to be checked, I would bring it up as a discussion on the asterisk-dev mailing list. Thanks! By: Russell Bryant (russell) 2006-10-17 11:20:18 This is a type of issue that would need to be handled on a case by case basis most likely.  Before working on any patches, if you aren't sure whether something needs to be checked, I would bring it up as a discussion on the asterisk-dev mailing list. Thanks! By: Markus Elfring (elfring) 2006-10-17 13:09:36 I would prefer to keep the issue open until all lines in the source files will be corrected. I guess that only two cases need to be distinguished to fix the danger. 1. graceful exit 2. abnormal program termination (abort) By: Russell Bryant (russell) 2006-10-17 20:16:38 The return value of memory allocation functions such as calloc and strdup should always be checked.  Patches to fix this are welcome. However, in Asterisk, we do not check the return value of ast_mutex_lock() on purpose.  That is because we are using recursive mutexes, so the operation will not fail. By: Markus Elfring (elfring) 2006-10-18 04:43:32 1. How much resources do the maintainers have got to fix such reported programming errors on their own? 2. You can not be 100% sure that a lock or unlock operation does not fail until the return value will be checked. Otherwise, you will not notice that something unexpected happened. By the way: Which functions do use mutexes recursively in the source code? (I guess that some parts can work without the option "PTHREAD_MUTEX_RECURSIVE".) By: Markus Elfring (elfring) 2006-10-18 07:57:31 2. The function "__ast_pthread_mutex_destroy" (file "http://svn.digium.com/view/asterisk/trunk/include/asterisk/lock.h?rev=43953&view=markup") uses incomplete error handling. The current logging approach may be wrong. An abort will be needed for correct program execution. Thread-safety will be lost if a non-zero value was returned. By: Russell Bryant (russell) 2006-10-23 20:30:02 All of the mutex's in Asterisk are recursive mutexes. I have fixed the places in muted.c and ael_main.c that did not check the return value of the memory allocation routines in both the 1.4 branch and the trunk in revisions 46067 and 46068.  Feel free to open a new report with any patches that you have to address further issues.  Thanks! By: Markus Elfring (elfring) 2006-10-24 08:19:57 I would appreciate if the calls of lock and unlock functions will also be fixed. By: Russell Bryant (russell) 2006-10-25 11:24:09 That issue is not a bug, it is intentional.  Please do not reopen this issue.