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

[Eric Engestrom]

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 :]

> 
> 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
> >
> >