2008/9/5 Francois Gouget fgouget@free.fr:
I have a few other concerns here:
- Why do we need a macro here? I thought it was so that
__builtin_object_size() could do its work, but the strcpy() functions above in the patch have no associated macro and they call __builtin_object_size() on their parameter which would seem to be poinless. So I must be misunderstanding something here.
- It's customary to add some extra parenthesing in such macros: (src),(srclen),__builtin_object_size((src),0),
- The macro itself can cause trouble in some cases. In Winelib
code it can cause naming collisions with user functions (especially with class methods, though it may not be very likely here), which would especially be an issue with strcpy*(). Maybe it should be protected by something like a WINE_NO_OVERFLOW_CHECK macro?
- The macro can also cause trouble in case its parameters have
side-effects, like x++ or similar (though the __builtin_object_size() mentions returning -1 in case or side-effects). This could impact Wine too.
I also have some questions: * Is there any runtime overhead of using __builtin_object_size and/or the __alloc_size__ attribute? * If not and it's a compile time only thing, why can't these buffer overruns be detected at compile time instead of runtime (which obviously depends on test coverage)?