Re: [Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
On Friday, 2019-04-12 11:50:11 -0400, Marek Olšák wrote: > On Fri, Apr 12, 2019 at 11:41 AM Eric Engestrom > wrote: > > > On Friday, 2019-04-12 11:00:56 -0400, Marek Olšák wrote: > > > On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli > > wrote: > > > > On 4/11/19 3:32 AM, Marek Olšák wrote: > > > > > - 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. > > > > What about `access(filename, F_OK)` ? > > > > That's not standard C API. Right, that's POSIX and that can't be used on windows, which this file can be built on. My bad :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
On Fri, Apr 12, 2019 at 11:41 AM Eric Engestrom wrote: > On Friday, 2019-04-12 11:00:56 -0400, Marek Olšák wrote: > > On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli > wrote: > > > On 4/11/19 3:32 AM, Marek Olšák wrote: > > > > - 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. > > What about `access(filename, F_OK)` ? > That's not standard C API. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
On Friday, 2019-04-12 11:00:56 -0400, Marek Olšák wrote: > On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli wrote: > > On 4/11/19 3:32 AM, Marek Olšák wrote: > > > - 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. What about `access(filename, F_OK)` ? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
On Thu, Apr 11, 2019 at 2:53 AM Tapani Pälli wrote:
>
> On 4/11/19 3:32 AM, Marek Olšák wrote:
> > From: Marek Olšák
> >
> > ---
> > 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
>
> > + 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",
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
On 4/11/19 3:32 AM, Marek Olšák wrote:
From: Marek Olšák
---
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.
- 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.
With or without the suggestion;
Reviewed-by: Tapani Pälli
+ 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",
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: don't overwrite existing shader files with MESA_SHADER_CAPTURE_PATH
From: Marek Olšák
---
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);
- file = fopen(filename, "w");
+ }
+ FILE *file = fopen(filename, "r");
+ if (!file)
+break;
+ 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",
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
