GNOME Bugzilla – Bug 135640
Usage of alloca is not portable, and breaks under NetBSD
Last modified: 2004-12-22 21:47:04 UTC
The file gst/interleave/interleave.c uses the alloca(3) function. Even though, the code assumes it's present, and also assumes the alloca.h header file exists. The later is the one causing problems in NetBSD, because the header file to use is stdlib.h, not the missing alloca.h. According to NetBSD's alloca(3) manpage: The alloca() function is machine dependent; its use is discouraged. The alloca() function is slightly unsafe because it cannot ensure that the pointer returned points to a valid and usable block of memory. The allocation made may exceed the bounds of the stack, or even go further into other objects in memory, and alloca() cannot determine such an error. Avoid alloca() with large unbounded allocations. Therefore, I suggest using malloc instead of alloca, since the former is standard and less problematic. alloca is used in just one place, so converting it to malloc should be easy (if I've understood alloca's functionality properly). The attached patch attempts to do this. If you still want to use alloca, then a check in configure is required. The right one to use is AC_FUNC_ALLOCA. Using it implies doing several other changes (and also implies having to provide a workaround in case it's missing, so we are back to switching to malloc), so I've not done this. See autoconf's info manual for more information about this topic. (Please note that the manual does not mention stdlib.h containing alloca's prototype, so you should #include this file in the source unconditionally to be sure it gets defined sometime, in case it's there and not in alloca.h.)
Created attachment 24873 [details] [review] Patch to switch from alloca to malloc
Applied, thanks.
The patch is somewhat OK. The thing with alloca is that it is really fast, so you can allocate memory from within the processing loop. The other choice is to allocate memory in the state-change function, or when a pad is added. I don't really consider malloc within the processing loop to be a viable option -- it is not realtime-safe, which is an eventual goal of gst. I don't think the author of the patch really understood this -- he failed to remove the comment describing alloca's use. So while the patch works, it is not correct. I will fix this later.
How about using g_alloca() here?
alloca() and all its derivatives are a hack. We're not going to use it in gstreamer. Any further discussion of this should be on the mailing list, and please address the list of issues I raised concerning alloca() first. I'm closing this, because the code is no longer buggy. If anyone wants to play with the code to make it neater, that's fine, but one doesn't need a bug for that.