D3D shader assembler

[Henri Verbeet]

2009/7/19 Matteo Bruni <matteo.mystral at gmail.com>:
> Hi to everybody,
> I'm sending here the main assembler patch for reviews and suggestions.
> It is bzipped as it is quite a big patch, but I couldn't find a
> meaningful way to split it.
It really is much easier to review as separate patches. The part that
follows is really only the first thing I noticed.

> +const char *debug_src(struct shader_reg *reg, BOOL vs) {
> +    static char buffer[128];
> +
> +    memset(buffer, 0, sizeof(buffer));
This really isn't acceptable:
  - "debug_src" isn't a very good name for something that visible
outside the current file
  - The reg parameter should be const.
  - Zeroing the entire buffer is quite unnecessary.
  - Just passing the shader type would be much better than a BOOL
parameter "vs" and hoping that anything that isn't a vertex shader is
a pixel shader. Even if it happens to be true.
  - "buffer" has a fixed size.
  - "buffer" is static. Using wine_dbg_sprintf() would already be much
better, but the entire "parsed_shader" setup looks flawed to me. It
seems to me that you should handle that by having an appropriately
filled struct asmparser_backend, and appending to a proper buffer.

Note that I think your mentor should have caught basic things like
this in a much earlier stage.