<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
On 4/11/19 3:32 AM, Marek Olšák wrote:<br>
> From: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank">marek.olsak@amd.com</a>><br>
> <br>
> ---<br>
>   src/mesa/main/shaderapi.c | 20 +++++++++++++++++---<br>
>   1 file changed, 17 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c<br>
> index 01342c04e8f..6b73e6c7e7a 100644<br>
> --- a/src/mesa/main/shaderapi.c<br>
> +++ b/src/mesa/main/shaderapi.c<br>
> @@ -1233,24 +1233,38 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,<br>
>            if (shProg->_LinkedShaders[stage])<br>
>               prog = shProg->_LinkedShaders[stage]->Program;<br>
>   <br>
>            _mesa_use_program(ctx, stage, shProg, prog, ctx->_Shader);<br>
>         }<br>
>      }<br>
>   <br>
>      /* Capture .shader_test files. */<br>
>      const char *capture_path = _mesa_get_shader_capture_path();<br>
>      if (shProg->Name != 0 && shProg->Name != ~0 && capture_path != NULL) {<br>
> -      FILE *file;<br>
> -      char *filename = ralloc_asprintf(NULL, "%s/%u.shader_test",<br>
> +      /* Find an unused filename. */<br>
> +      char *filename = NULL;<br>
> +      for (unsigned i = 0;; i++) {<br>
> +         if (i) {<br>
> +            filename = ralloc_asprintf(NULL, "%s/%u-%u.shader_test",<br>
> +                                       capture_path, shProg->Name, i);<br>
> +         } else {<br>
> +            filename = ralloc_asprintf(NULL, "%s/%u.shader_test",<br>
>                                          capture_path, shProg->Name);<br>
<br>
How about just having the counter always there, to simplify a bit and <br>
have consistent filename scheme? Just a suggestion.<br></blockquote><div><br></div><div>My personal preference is to keep the original name without the -%u suffix if %u == 0.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> -      file = fopen(filename, "w");<br>
> +         }<br>
> +         FILE *file = fopen(filename, "r");<br>
> +         if (!file)<br>
> +            break;<br>
<br>
I'm surprised we don't have some helper like 'util_path_exists' but this <br>
works, I guess then we should have 'util_path_isdir|isfile' and others <br>
as well.<br></blockquote><div><br></div><div>There is no standard API for checking whether a file exists. fopen is the only standard way to do it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
With or without the suggestion;<br>
Reviewed-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>><br>
<br>
> +         fclose(file);<br>
> +         ralloc_free(filename);<br>
> +      }<br>
> +<br>
> +      FILE *file = fopen(filename, "w");<br>
>         if (file) {<br>
>            fprintf(file, "[require]\nGLSL%s >= %u.%02u\n",<br>
>                    shProg->IsES ? " ES" : "",<br>
>                    shProg->data->Version / 100, shProg->data->Version % 100);<br>
>            if (shProg->SeparateShader)<br>
>               fprintf(file, "GL_ARB_separate_shader_objects\nSSO ENABLED\n");<br>
>            fprintf(file, "\n");<br>
>   <br>
>            for (unsigned i = 0; i < shProg->NumShaders; i++) {<br>
>               fprintf(file, "[%s shader]\n%s\n",<br>
> <br>
</blockquote></div></div>