Re: [v2 4/7] d3dx9: Add scalar and vector setting path to set_constants().

[Matteo Bruni]

2017-06-12 13:22 GMT+02:00 Paul Gofman :
> Signed-off-by: Paul Gofman
>
> dlls/d3dx9_36/preshader.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
> index 2f9648d..1ca02c4 100644
> --- a/dlls/d3dx9_36/preshader.c
> +++ b/dlls/d3dx9_36/preshader.c
> @@ -1101,6 +1101,12 @@ static void regstore_set_data(struct d3dx_regstore *rs, unsigned int table,
>      };
>      enum pres_value_type table_type = table_info[table].type;
> - if (param_type == table_type)
> - {
> -
>    regstore_set_values(rs, table, in, offset, count);
> -
>    return;
> - }

Maybe it's a bit paranoid but I'd like an assert in
regstore_set_values() ensuring that there is no overlap between
destination and source of the memcpy(). That's clearly the case at the
moment but it might become less obvious in the future.

[v2 4/7] d3dx9: Add scalar and vector setting path to set_constants().

[Paul Gofman]

On 06/14/2017 02:59 AM, Matteo Bruni wrote:
> 2017-06-12 13:22 GMT+02:00 Paul Gofman :
> > Signed-off-by: Paul Gofman
> >
> > dlls/d3dx9_36/preshader.c | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
> > index 2f9648d..1ca02c4 100644
> > --- a/dlls/d3dx9_36/preshader.c
> > +++ b/dlls/d3dx9_36/preshader.c
> > @@ -1101,6 +1101,12 @@ static void regstore_set_data(struct d3dx_regstore *rs, unsigned int table,
> >       };
> >       enum pres_value_type table_type = table_info[table].type;
> > - if (param_type == table_type)
> > - {
> > -
> >    regstore_set_values(rs, table, in, offset, count);
> > -
> >    return;
> > - }
>
> Maybe it's a bit paranoid but I'd like an assert in
> regstore_set_values() ensuring that there is no overlap between
> destination and source of the memcpy(). That's clearly the case at the
> moment but it might become less obvious in the future.

Maybe just change memcpy() to memmove() then? Or assert for pointers 
equality instead of memory regions overlap?

[v2 4/7] d3dx9: Add scalar and vector setting path to set_constants().

[Matteo Bruni]

2017-06-14 2:32 GMT+02:00 Paul Gofman :
> On 06/14/2017 02:59 AM, Matteo Bruni wrote:
> > 2017-06-12 13:22 GMT+02:00 Paul Gofman :
> > > Signed-off-by: Paul Gofman
> > >
> > > dlls/d3dx9_36/preshader.c | 23 +++++++++++++++++++++++
> > >   1 file changed, 23 insertions(+)
> > >
> > > diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
> > > index 2f9648d..1ca02c4 100644
> > > --- a/dlls/d3dx9_36/preshader.c
> > > +++ b/dlls/d3dx9_36/preshader.c
> > > @@ -1101,6 +1101,12 @@ static void regstore_set_data(struct d3dx_regstore
> > > *rs, unsigned int table,
> > >       };
> > >       enum pres_value_type table_type = table_info[table].type;
> > > - if (param_type == table_type)
> > > - {
> > > -
> > >    regstore_set_values(rs, table, in, offset, count);
> > > -
> > >    return;
> > > - }
> >
> > Maybe it's a bit paranoid but I'd like an assert in
> > regstore_set_values() ensuring that there is no overlap between
> > destination and source of the memcpy(). That's clearly the case at the
> > moment but it might become less obvious in the future.
>
> Maybe just change memcpy() to memmove() then? Or assert for pointers
> equality instead of memory regions overlap?

Nah, memcpy() is fine and potentially faster. Simply checking for
pointer equality is probably enough, although checking for overlap
shouldn't be too terrible (something like assert((src < dst && src +
size <= dst) || (src > dst && src >= dst + size) should work).