Summary:	ASTERISK-00577: [patch] Remove a little stack abuse from format_jpeg.c
Reporter:	dw (dw)	Labels:
Date Opened:	2003-11-26 15:47:07.000-0600	Date Closed:	2011-06-07 14:11:55
Priority:	Trivial	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) format_jpeg.c.real.patch
Description:	There is a 64k automatic variable allocated in format_jpeg.c. This patch removes that. Also note this means there is no longer a hard limit of 64k on JPEGs, which could be a good or a bad thing. David.
Comments:	By: Tilghman Lesher (tilghman) 2003-11-30 10:02:33.000-0600 Instead of using malloc() to allocate memory that does not last beyond the function, use alloca(). By: dw (dw) 2003-11-30 10:37:44.000-0600 What is your rationale for this? alloca is bad on a few fronts, namely:    a) It is not portable to everywhere. This doesn't matter much since portability doesn't seem to be a goal of Asterisk at present.    b) It is a dangerous interface - if your stack is full, alloca will cause a SEGV rather than return an error. The heap is much more suited for large amounts of data, and from a professional standpoint, loading a large binary object into your stack is insane. alloca provides a fast performing, syntactically clean method to get a chunk of memory from the stack. In this day and age, the performance benefit is essentially null, and syntactic preference is a sign of bad programming. I stand by my choice to use malloc(). :) David. By: jerjer (jerjer) 2003-11-30 10:44:16.000-0600 man alloca: BUGS       The alloca function is machine and compiler dependent. Its use is discouraged.       On many systems alloca cannot be used inside the list of arguments of a function call, because the stack  space reserved by alloca would appear on the stack in the middle of the space for the function arguments. By: Tilghman Lesher (tilghman) 2003-11-30 13:13:06.000-0600 Personally, I never used alloca() before submitting patches for Asterisk, but I'm telling you to use it, because that's what it will take for your patch to be accepted into CVS. Mark likes it; Mark has control over what goes into CVS; therefore, use alloca().  ;-) By: Brian West (bkw918) 2003-11-30 13:19:50.000-0600 memset anyone? By: dw (dw) 2003-11-30 17:14:05.000-0600 Rejecting this patch due to personal preference really isn't on - alloca _is_ dangerous and I'm not rewriting the patch to be broken. Allocating an arbitrary amount of stack to a user-supplied object is completely stupid - as I previously noted ("which could be a good or a bad thing."), even doing this on the heap is bad. I don't use Asterisk, and I possibly never will now. I came across the bug during a quick review of the source to see whether or not it was suitable for use in our business. I clearly see now, that it is not. David. By: Brian West (bkw918) 2003-12-01 12:15:42.000-0600 Not a bug