On 15.07.2015 16:35, Alexandre Julliard wrote:
Sebastian Lackner sebastian@fds-team.de writes:
I am not sure why you think it is questionable. We have compiler support for variadic functions, so why should we not use it? Please note that on x86_64 this does more than just grabbing a pointer to the variadic arguments, the declaration as WINAPIV also ensures that the compiler stores the register values ecx/edx/r8/r9 back onto the stack:
If you need them on the stack, you should be putting them there from assembly, not relying on the compiler implementation of va_list or assuming that va_list is a pointer to the first argument.
va_list is supposed to be an opaque type, so if you want to use it you'd have to access the arguments from C code too, which AFAICS is not possible in this case.
Wine contains several public functions which take __ms_va_list pointers as arguments. Its well-defined what such a __ms_va_list means, and it will never change because it would break the ABI. I also think that we can rely on the fact that compilers work properly, the values on the stack of course have to be initialized before the next function call, otherwise it would be a compiler bug.
Wine contains mixed C/Assembly code at a lot of places, and its nothing unusual that the Assembly code makes assumptions about how data structures are stored / passed. Why is it a problem here? Even when I add additional Assembler functions and wrapper, they will still contain ABI specific assumptions.
Moreover, implementing the whole logic of _vcomp_fork in Assembler seems unreasonable. Do you mean a second Assembler wrapper, one to grab the "valist" pointer, and one to copy the arguments again on the target stack to execute the function?
Probably something like that, or something along the lines of the existing relay code.
I will take a closer look, but I am not convinced that this will make the code look better or improve anything. I would still prefer if the patch would be accepted in the original shape because thats the best way to do it, imho.
Best regards, Sebastian