Friday, 30 August 2013

Another failed patch

Whilst doing the long overdue merge of trunk into my invoke-diff-cmd branch, I spotted a deprecation warning that looked like a minor oversight, and so, I fixed it, svn compiled, the tests passed and ... the patch turned out to be a no-can-do, because APR_ARRAY_PUSH doesn't care what type you stuff in there.

I thought it did, because of this:

#define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push(ary))) 

which *looks* like it's type-safe at least if you don't know what you're really looking at (and if you haven't bothered to check the underlying code to see what it actually does).  It of course does no such thing -- so, what does this trick that does seemingly nothing actually do?

Ask on #svn-dev and ye shall receive:

[19:32] <breser> gbb: The reason it takes a type is to avoid warnings.  The apr_array_push() function returns a void *.  Technically there's no issue with putting any pointer in a void pointer, however some  compilers will warn you about that since you may not be doing it intentionally.                                                        
[19:33] <breser> gbb: The array code doesn't care what the pointer is  to, it's up to the user of APR Arrays to keep their types straight.   
[19:34] <breser> gbb: In this case the fact that the array code is  taking a type (and casting) is hiding the errors/warnings you would have otherwise seen.                                                                                                                     
[20:30] <Bert> breser, gbb: Apr arrays can be of different kinds than just pointers. In some cases we have arrays of specific struct kinds  (e.g. for property changes in the diff handling) 

Thank you Ben and Bert, it certainly did clear things up, and made me look up the 'long answer' --- the very short answer of course is that C isn't C++, and so doesn't have type-safe templates.  Caveat coder!