[Mesa-dev] [PATCH mesa] anv: return true only if all the writes succeed, not just the last one

[Jason Ekstrand]

On Thu, May 31, 2018 at 8:54 AM, Eric Engestrom <eric.engestrom at intel.com>
wrote:

> On Tuesday, 2018-05-29 09:55:05 -0700, Jason Ekstrand wrote:
> > Does this fix something?  If I understand correctly, the first blob_write
> > to fail will set out_of_memory and all subsequent blob_writes will fail.
>
> You're right actually, I didn't look deep enough.
>
> > Does this fix something?
>
> My clang complained about ok being overwritten without being read, and
> my shallow investigation didn't catch the out_of_memory flag in
> grow_to_fit().
>
> I guess we could drop `ok` and all its assignments, and simply return
> the last blob_write_bytes() call, or I could just drop it as it's not
> really important as it turns out there isn't any actual bug :]
>

Sounds like a plan. :)


> >
> > On Tue, May 29, 2018 at 9:12 AM, Eric Engestrom <
> eric.engestrom at intel.com>
> > wrote:
> >
> > > Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
> > > ---
> > >  src/intel/vulkan/anv_pipeline_cache.c | 38
> +++++++++++++--------------
> > >  1 file changed, 19 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/src/intel/vulkan/anv_pipeline_cache.c
> b/src/intel/vulkan/anv_
> > > pipeline_cache.c
> > > index 82551e9f81f293ecadbe..6c0edaf1ec1b773802e1 100644
> > > --- a/src/intel/vulkan/anv_pipeline_cache.c
> > > +++ b/src/intel/vulkan/anv_pipeline_cache.c
> > > @@ -96,29 +96,29 @@ static bool
> > >  anv_shader_bin_write_to_blob(const struct anv_shader_bin *shader,
> > >                               struct blob *blob)
> > >  {
> > > -   bool ok;
> > > +   bool ok = true;
> > >
> > > -   ok = blob_write_uint32(blob, shader->key->size);
> > > -   ok = blob_write_bytes(blob, shader->key->data, shader->key->size);
> > > +   ok &= blob_write_uint32(blob, shader->key->size);
> > > +   ok &= blob_write_bytes(blob, shader->key->data, shader->key->size);
> > >
> > > -   ok = blob_write_uint32(blob, shader->kernel_size);
> > > -   ok = blob_write_bytes(blob, shader->kernel.map,
> shader->kernel_size);
> > > +   ok &= blob_write_uint32(blob, shader->kernel_size);
> > > +   ok &= blob_write_bytes(blob, shader->kernel.map,
> shader->kernel_size);
> > >
> > > -   ok = blob_write_uint32(blob, shader->prog_data_size);
> > > -   ok = blob_write_bytes(blob, shader->prog_data,
> shader->prog_data_size);
> > > -   ok = blob_write_bytes(blob, shader->prog_data->param,
> > > -                               shader->prog_data->nr_params *
> > > -                               sizeof(*shader->prog_data->param));
> > > +   ok &= blob_write_uint32(blob, shader->prog_data_size);
> > > +   ok &= blob_write_bytes(blob, shader->prog_data,
> > > shader->prog_data_size);
> > > +   ok &= blob_write_bytes(blob, shader->prog_data->param,
> > > +                                shader->prog_data->nr_params *
> > > +                                sizeof(*shader->prog_data->param));
> > >
> > > -   ok = blob_write_uint32(blob, shader->bind_map.surface_count);
> > > -   ok = blob_write_uint32(blob, shader->bind_map.sampler_count);
> > > -   ok = blob_write_uint32(blob, shader->bind_map.image_count);
> > > -   ok = blob_write_bytes(blob, shader->bind_map.surface_to_
> descriptor,
> > > -                               shader->bind_map.surface_count *
> > > -                               sizeof(*shader->bind_map.
> > > surface_to_descriptor));
> > > -   ok = blob_write_bytes(blob, shader->bind_map.sampler_to_
> descriptor,
> > > -                               shader->bind_map.sampler_count *
> > > -                               sizeof(*shader->bind_map.
> > > sampler_to_descriptor));
> > > +   ok &= blob_write_uint32(blob, shader->bind_map.surface_count);
> > > +   ok &= blob_write_uint32(blob, shader->bind_map.sampler_count);
> > > +   ok &= blob_write_uint32(blob, shader->bind_map.image_count);
> > > +   ok &= blob_write_bytes(blob, shader->bind_map.surface_to_
> descriptor,
> > > +                                shader->bind_map.surface_count *
> > > +                                sizeof(*shader->bind_map.
> > > surface_to_descriptor));
> > > +   ok &= blob_write_bytes(blob, shader->bind_map.sampler_to_
> descriptor,
> > > +                                shader->bind_map.sampler_count *
> > > +                                sizeof(*shader->bind_map.
> > > sampler_to_descriptor));
> > >
> > >     return ok;
> > >  }
> > > --
> > > Cheers,
> > >   Eric
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180531/326f219b/attachment-0001.html>