Re: [1/3] d3dx9: Partial implementation of D3DXAssembleShader function.

[Henri Verbeet]

On 2 February 2010 18:11, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
> +void wpp_write_message_var(const char *fmt, ...)
> This is probably not strictly required, but if you add the gcc format
> attribute to the function, gcc will check if the parameters types
> match the ones in the format string. You can look at the PRINTF_ATTR
> macro in dlls/wined3d/wined3d_private and similar macros in other
> places for examples.
> +        if(current_include == NULL)
> +        {
> +            ERR("current_include == NULL, desc == %p, buffer = %s\n",
> +                desc, desc->buffer);
> +            HeapFree(GetProcessHeap(), 0, desc);
> +            return;
> +        }
> +        ID3DXInclude_Close(current_include, desc->buffer);
> +        HeapFree(GetProcessHeap(), 0, desc);
> +        return;
> Something like the following is probably nicer:

+        if (current_include)
+            ID3DXInclude_Close(current_include, desc->buffer);
+        else
+            ERR("current_include == NULL, desc == %p, buffer = %s\n",
desc, desc->buffer);
+
+        HeapFree(GetProcessHeap(), 0, desc);
+        return;
> +    struct wpp_callbacks wpp_callbacks = {
> +        wpp_lookup_mem,
> +        wpp_open_mem,
> +        wpp_close_mem,
> +        wpp_read_mem,
> +        wpp_write_mem,
> +        wpp_error,
> +        wpp_warning,
> +    };
> This should be static const.
> +            TRACE("Shader source:\n");
> +            for(i=0;i<data_len;i++)
> +            {
> +                TRACE("%c", data[i]);
> +            }
> +            TRACE("\n");
> This doesn't look right. You should be using debugstr_an(), since this
> is external data you're tracing.