Cleaning up the source code

Although I originally limited myself to cleaning up code in xine-lib 1.2 branch, I’ve started lately to care a bit more about the 1.1 branch too. The reason for this is that the last few security issues we had were already fixed in the 1.2 series by my calloc() changes (suggested to me by our great Gentoo Security team more than an year ago), which are now backported.

While I took a pause in the development of the new qt/mpeg4 demuxer (which I’m sure will be quite more solid than the previous one) I’ve resumed removing xine_xmalloc calls from the code, hopefully soon it will be gone from xine-lib itself at least. I’ll have to check the frontends to see if they do that too.

The interesting part of this job is to actually compare how the size of the code changes when you make some changes around. For instance, when I change a xine_xmalloc() call with a calloc() call (because the latter zeroes out the memory area, while malloc() gives it pristine), the size of the code increase, as the call has two parameters rather than one (in a lot of cases calloc is called with 1 as first parameter). This shouldn’t be a problem considering that xine_xmalloc() also calls calloc(), so it’s likely to execute less instructions at the end anyway. On the other hand, I don’t like increasing codesize if I can, so I started doing related cleaning so that strndup() is used instead of allocating the space and then filling it with strcpy(), or using asprintf() rather than allocating and then using sprintf(), which allowed me to stay about at the same codesize even though xine_xmalloc() was replaced.

One thing that scares me is that a lot of time the size of the memory area to allocate is first computed in an integer variable. A signed integer variable. You can guess that it isn’t really so safe to do that as you can easily get an overflow on those variables, and then who knows what would happen. Unfortunately I can’t find way for gcc to warn me whenever an alloc function is called with signed parameters. I always try to use size_t when necessary though.

And instead, a common problem that really needs to be addressed in some way is the fact that a lot of places tend to initialise to zero a memory area right after allocating it. Even when they used xine_xmalloc() (that always zeroed out the area) and now when they use calloc() (which also does that). The mild cases of uselessness use(d) memset() to do their job, which wouldn’t have been that bad, but a few places iterates over the allocated memory area to set it by hand. Which is very much a waste of time and code.