Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2 / zend_API.c

At 13:05 03/08/2003, Marcus Börger wrote:

The thing i need to initialize is at least op_array.opcodes but i think that any pointer should be initialized to NULL in such structs. And when i looked into the stucts during debugging i thought that it would make more sense to zero initialize the whole struct instead of searching for some elements.

Ok, I looked a bit more into the patch and where there's smoke there's usually fire - it's one of those 'magical' patches... In practice, what your patches do is look at the .opcodes pointer without first verifying that they're dealing with a userland function. That's a bug. It's happening all too often lately (done by many people in various different forms, including even basic zval handling), so let me repeat it one more time: For unions, one MUST NOT touch any element before checking the union's type. In this case, if you first verified that you're dealing with a userland function, the .opcodes element is guaranteed to be valid (since it's initialized by init_op_array()). The only elements you may touch without checking the type first are located in the 'common' struct. Before you say "but it works!", let me tell you that it works out of sheer luck, and may stop working in the future without any prior warnings. The reason is that you rely on the fact that the zend_internal_function doesn't have an element that shares the same storage as the opcodes element of zend_op_array. If tomorrow we move 'opcodes' upwards, or add more elements to the internal_function struct, you initialization magic would stop working. See my point about covering errors instead of finding them? :) Again, if there's one thing I'd like everybody to remember from this is ALWAYS check the types before you deal with union elements. Zeev