[Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH

[Marek Olšák]

On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli <tapani.palli at intel.com> wrote:

>
> On 4/11/19 3:32 AM, Marek Olšák wrote:
> > From: Marek Olšák <marek.olsak at amd.com>
> >
> > ---
> >   src/mesa/main/shaderapi.c | 20 +++++++++++++++++---
> >   1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> > index 01342c04e8f..6b73e6c7e7a 100644
> > --- a/src/mesa/main/shaderapi.c
> > +++ b/src/mesa/main/shaderapi.c
> > @@ -1233,24 +1233,38 @@ link_program(struct gl_context *ctx, struct
> gl_shader_program *shProg,
> >            if (shProg->_LinkedShaders[stage])
> >               prog = shProg->_LinkedShaders[stage]->Program;
> >
> >            _mesa_use_program(ctx, stage, shProg, prog, ctx->_Shader);
> >         }
> >      }
> >
> >      /* Capture .shader_test files. */
> >      const char *capture_path = _mesa_get_shader_capture_path();
> >      if (shProg->Name != 0 && shProg->Name != ~0 && capture_path !=
> NULL) {
> > -      FILE *file;
> > -      char *filename = ralloc_asprintf(NULL, "%s/%u.shader_test",
> > +      /* Find an unused filename. */
> > +      char *filename = NULL;
> > +      for (unsigned i = 0;; i++) {
> > +         if (i) {
> > +            filename = ralloc_asprintf(NULL, "%s/%u-%u.shader_test",
> > +                                       capture_path, shProg->Name, i);
> > +         } else {
> > +            filename = ralloc_asprintf(NULL, "%s/%u.shader_test",
> >                                          capture_path, shProg->Name);
>
> How about just having the counter always there, to simplify a bit and
> have consistent filename scheme? Just a suggestion.
>

My personal preference is to keep the original name without the -%u suffix
if %u == 0.


>
> > -      file = fopen(filename, "w");
> > +         }
> > +         FILE *file = fopen(filename, "r");
> > +         if (!file)
> > +            break;
>
> I'm surprised we don't have some helper like 'util_path_exists' but this
> works, I guess then we should have 'util_path_isdir|isfile' and others
> as well.
>

There is no standard API for checking whether a file exists. fopen is the
only standard way to do it.


>
> With or without the suggestion;
> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
>
> > +         fclose(file);
> > +         ralloc_free(filename);
> > +      }
> > +
> > +      FILE *file = fopen(filename, "w");
> >         if (file) {
> >            fprintf(file, "[require]\nGLSL%s >= %u.%02u\n",
> >                    shProg->IsES ? " ES" : "",
> >                    shProg->data->Version / 100, shProg->data->Version %
> 100);
> >            if (shProg->SeparateShader)
> >               fprintf(file, "GL_ARB_separate_shader_objects\nSSO
> ENABLED\n");
> >            fprintf(file, "\n");
> >
> >            for (unsigned i = 0; i < shProg->NumShaders; i++) {
> >               fprintf(file, "[%s shader]\n%s\n",
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190412/ce52afbc/attachment-0001.html>