[Piglit] [PATCH] util: use piglit_load_text_file() in piglit_compile_shader()

Jamey Sharp jamey at minilop.net
Mon Apr 28 14:50:28 PDT 2014


Looks good to me! I don't know yet how the merge process works for
Piglit, but I'm happy to offer my:

Reviewed-by: Jamey Sharp <jamey at minilop.net>

Jamey

On Mon, Apr 28, 2014 at 2:46 PM, Brian Paul <brianp at vmware.com> wrote:
> The old code had a problem on MinGW.  If the shader/text file had
> DOS-style \r\n line endings, fread() would convert them to Unix-style
> \n line endings.  Since the actual number of chars read by fread()
> was less than the stat()'d size, we put the terminating '\0' in the
> wrong place, possibly after some garbage characters in the buffer.
>
> This sometimes caused the GLSL compiler to generate an error when it
> found those garbage chars.
>
> A Heisenbug:  I was seeing failures w/out gdb but success w/ gdb. Ugh!
>
> v2: get rid of stat() call too, per Jamey Sharp.
> ---
>  tests/util/piglit-shader.c |   29 ++++++-----------------------
>  1 file changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/tests/util/piglit-shader.c b/tests/util/piglit-shader.c
> index b326abd..76a4d03 100644
> --- a/tests/util/piglit-shader.c
> +++ b/tests/util/piglit-shader.c
> @@ -21,7 +21,6 @@
>   * USE OR OTHER DEALINGS IN THE SOFTWARE.
>   */
>
> -#include <sys/stat.h>
>  #include <errno.h>
>
>  #include "piglit-util-gl-common.h"
> @@ -66,10 +65,7 @@ GLuint
>  piglit_compile_shader(GLenum target, const char *filename)
>  {
>         GLuint prog;
> -       struct stat st;
> -       int err;
>         GLchar *prog_string;
> -       FILE *f;
>         const char *source_dir;
>         char filename_with_path[FILENAME_MAX];
>
> @@ -82,28 +78,15 @@ piglit_compile_shader(GLenum target, const char *filename)
>                  "%s/tests/%s", source_dir, filename);
>         filename_with_path[FILENAME_MAX - 1] = 0;
>
> -       err = stat(filename_with_path, &st);
> -       if (err == -1) {
> -               fprintf(stderr, "Couldn't stat program %s: %s\n", filename_with_path, strerror(errno));
> -               fprintf(stderr, "You can override the source dir by setting the PIGLIT_SOURCE_DIR environment variable.\n");
> +       prog_string = piglit_load_text_file(filename_with_path, NULL);
> +       if (!prog_string) {
> +               fprintf(stderr, "Couldn't read shader %s: %s\n",
> +                       filename_with_path, strerror(errno));
> +               fprintf(stderr, "You can override the source dir by setting the"
> +                       " PIGLIT_SOURCE_DIR environment variable.\n");
>                 exit(1);
>         }
>
> -       prog_string = malloc(st.st_size + 1);
> -       if (prog_string == NULL) {
> -               fprintf(stderr, "malloc\n");
> -               exit(1);
> -       }
> -
> -       f = fopen(filename_with_path, "r");
> -       if (f == NULL) {
> -               fprintf(stderr, "Couldn't open program: %s\n", strerror(errno));
> -               exit(1);
> -       }
> -       fread(prog_string, 1, st.st_size, f);
> -       prog_string[st.st_size] = '\0';
> -       fclose(f);
> -
>         prog = piglit_compile_shader_text(target, prog_string);
>
>         free(prog_string);
> --
> 1.7.10.4
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


More information about the Piglit mailing list