[Mesa-dev] mesa: Fix substitution of large shaders

Submitted by Cody Northrop on June 6, 2014, 7:04 p.m.

Details

Message ID CAP90KKdHQ75Ka+XTX5UZ7hffF+Ha5enPGDfvYh8XxAseBWPu8w@mail.gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Cody Northrop June 6, 2014, 7:04 p.m.
Whoops, yes, you're right.  ftell() of SEEK_END and strlen() are returning
the same value, which does not include the terminating zero.  Updated patch
below.

We use this quite a bit, much faster than editing large apitrace files

Thanks,

-C


Signed-off-by: Cody Northrop <cody@lunarg.com>
---
 src/mesa/main/shaderapi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 6f84acd..4048ddf 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -1392,7 +1392,7 @@  _mesa_LinkProgram(GLhandleARB programObj)
 static GLcharARB *
 read_shader(const char *fname)
 {
-   const int max = 50*1000;
+   int shader_size = 0;
    FILE *f = fopen(fname, "r");
    GLcharARB *buffer, *shader;
    int len;
@@ -1401,8 +1401,19 @@  read_shader(const char *fname)
       return NULL;
    }

-   buffer = malloc(max);
-   len = fread(buffer, 1, max, f);
+   /* allocate enough room for the entire shader */
+   fseek(f, 0, SEEK_END);
+   shader_size = ftell(f);
+   rewind(f);
+   assert(shader_size);
+
+   /* add one for terminating zero */
+   shader_size++;
+
+   buffer = malloc(shader_size);
+   assert(buffer);
+
+   len = fread(buffer, 1, shader_size, f);
    buffer[len] = 0;

    fclose(f);

Comments

Reviewed-by: Brian Paul <brianp@vmware.com>

Do you need someone to commit/push this for you?

-Brian

On 06/06/2014 12:04 PM, Cody Northrop wrote:
> Whoops, yes, you're right.  ftell() of SEEK_END and strlen() are
> returning the same value, which does not include the terminating zero.
> Updated patch below.
>
> We use this quite a bit, much faster than editing large apitrace files
>
> Thanks,
>
> -C
>
>
> Signed-off-by: Cody Northrop <cody@lunarg.com <mailto:cody@lunarg.com>>
> ---
>   src/mesa/main/shaderapi.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 6f84acd..4048ddf 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1392,7 +1392,7 @@ _mesa_LinkProgram(GLhandleARB programObj)
>   static GLcharARB *
>   read_shader(const char *fname)
>   {
> -   const int max = 50*1000;
> +   int shader_size = 0;
>      FILE *f = fopen(fname, "r");
>      GLcharARB *buffer, *shader;
>      int len;
> @@ -1401,8 +1401,19 @@ read_shader(const char *fname)
>         return NULL;
>      }
>
> -   buffer = malloc(max);
> -   len = fread(buffer, 1, max, f);
> +   /* allocate enough room for the entire shader */
> +   fseek(f, 0, SEEK_END);
> +   shader_size = ftell(f);
> +   rewind(f);
> +   assert(shader_size);
> +
> +   /* add one for terminating zero */
> +   shader_size++;
> +
> +   buffer = malloc(shader_size);
> +   assert(buffer);
> +
> +   len = fread(buffer, 1, shader_size, f);
>      buffer[len] = 0;
>
>      fclose(f);
> --
> 1.8.3.2
>
>
>
>
>
>
> On Thu, Jun 5, 2014 at 7:13 PM, Brian Paul <brianp@vmware.com
> <mailto:brianp@vmware.com>> wrote:
>
>     On 06/05/2014 10:47 AM, Cody Northrop wrote:
>
>         The fixed size is insufficient for shaders I'm debugging.
>           Rather than
>         just bump it up, make it dynamic.
>
>         Thanks,
>
>         -C
>
>         Signed-off-by: Cody Northrop <cody@lunarg.com
>         <mailto:cody@lunarg.com> <mailto:cody@lunarg.com
>         <mailto:cody@lunarg.com>>>
>
>         ---
>            src/mesa/main/shaderapi.c | 14 +++++++++++---
>            1 file changed, 11 insertions(+), 3 deletions(-)
>
>         diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>         index 6f84acd..e63c124 100644
>         --- a/src/mesa/main/shaderapi.c
>         +++ b/src/mesa/main/shaderapi.c
>         @@ -1392,7 +1392,7 @@ _mesa_LinkProgram(GLhandleARB programObj)
>            static GLcharARB *
>            read_shader(const char *fname)
>            {
>         -   const int max = 50*1000;
>         +   int shader_size = 0;
>               FILE *f = fopen(fname, "r");
>               GLcharARB *buffer, *shader;
>               int len;
>         @@ -1401,8 +1401,16 @@ read_shader(const char *fname)
>                  return NULL;
>               }
>
>         -   buffer = malloc(max);
>         -   len = fread(buffer, 1, max, f);
>         +   /* allocate enough room for the entire shader */
>         +   fseek(f, 0, SEEK_END);
>         +   shader_size = ftell(f);
>         +   rewind(f);
>         +   assert(shader_size);
>         +
>         +   buffer = malloc(shader_size);
>
>
>     Do you have to add one for the terminating zero?
>
>
>
>         +   assert(buffer);
>         +
>         +   len = fread(buffer, 1, shader_size, f);
>               buffer[len] = 0;
>
>               fclose(f);
>         --
>
>
>     I thought I was the only person who ever used this code!
>
>     Other than the one question above this looks alright.
>
>     Reviewed-by: Brian Paul <brianp@vmware.com <mailto:brianp@vmware.com>>
>
>     _________________________________________________
>     mesa-dev mailing list
>     mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
>     http://lists.freedesktop.org/__mailman/listinfo/mesa-dev
>     <https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=BQB5tezP6JJarG3K6deQKB%2FgjK%2FGhKQvd8LRF8usgCs%3D%0A&s=6d587c1f9718375e59390ea001cad3ee836a3597c86c77596f02292c818b3051>
>
>
>
>
> --
>   Cody Northrop
>   Graphics Software Engineer
>   LunarG, Inc.- 3D Driver Innovations
>   Email: cody@lunarg.com <mailto:cody@lunarg.com>
>   Website: http://www.lunarg.com
> <https://urldefense.proofpoint.com/v1/url?u=http://www.lunarg.com/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=BQB5tezP6JJarG3K6deQKB%2FgjK%2FGhKQvd8LRF8usgCs%3D%0A&s=e4c25bb2e76d3a5cbff56061766e2be646a3106ae813979beea26ede2680e7ec>
Hi Brian,

I think I've pushed this patch. Could you verify that I haven't broken
anything?

Thanks!
Courtney


On Fri, Jun 6, 2014 at 7:20 PM, Brian Paul <brianp@vmware.com> wrote:

>
> Reviewed-by: Brian Paul <brianp@vmware.com>
>
> Do you need someone to commit/push this for you?
>
> -Brian
>
>
> On 06/06/2014 12:04 PM, Cody Northrop wrote:
>
>> Whoops, yes, you're right.  ftell() of SEEK_END and strlen() are
>> returning the same value, which does not include the terminating zero.
>> Updated patch below.
>>
>> We use this quite a bit, much faster than editing large apitrace files
>>
>> Thanks,
>>
>> -C
>>
>>
>> Signed-off-by: Cody Northrop <cody@lunarg.com <mailto:cody@lunarg.com>>
>>
>> ---
>>   src/mesa/main/shaderapi.c | 17 ++++++++++++++---
>>   1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index 6f84acd..4048ddf 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -1392,7 +1392,7 @@ _mesa_LinkProgram(GLhandleARB programObj)
>>   static GLcharARB *
>>   read_shader(const char *fname)
>>   {
>> -   const int max = 50*1000;
>> +   int shader_size = 0;
>>      FILE *f = fopen(fname, "r");
>>      GLcharARB *buffer, *shader;
>>      int len;
>> @@ -1401,8 +1401,19 @@ read_shader(const char *fname)
>>         return NULL;
>>      }
>>
>> -   buffer = malloc(max);
>> -   len = fread(buffer, 1, max, f);
>> +   /* allocate enough room for the entire shader */
>> +   fseek(f, 0, SEEK_END);
>> +   shader_size = ftell(f);
>> +   rewind(f);
>> +   assert(shader_size);
>> +
>> +   /* add one for terminating zero */
>> +   shader_size++;
>> +
>> +   buffer = malloc(shader_size);
>> +   assert(buffer);
>> +
>> +   len = fread(buffer, 1, shader_size, f);
>>      buffer[len] = 0;
>>
>>      fclose(f);
>> --
>> 1.8.3.2
>>
>>
>>
>>
>>
>>
>> On Thu, Jun 5, 2014 at 7:13 PM, Brian Paul <brianp@vmware.com
>> <mailto:brianp@vmware.com>> wrote:
>>
>>     On 06/05/2014 10:47 AM, Cody Northrop wrote:
>>
>>         The fixed size is insufficient for shaders I'm debugging.
>>           Rather than
>>         just bump it up, make it dynamic.
>>
>>         Thanks,
>>
>>         -C
>>
>>         Signed-off-by: Cody Northrop <cody@lunarg.com
>>         <mailto:cody@lunarg.com> <mailto:cody@lunarg.com
>>
>>         <mailto:cody@lunarg.com>>>
>>
>>         ---
>>            src/mesa/main/shaderapi.c | 14 +++++++++++---
>>            1 file changed, 11 insertions(+), 3 deletions(-)
>>
>>         diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>>         index 6f84acd..e63c124 100644
>>         --- a/src/mesa/main/shaderapi.c
>>         +++ b/src/mesa/main/shaderapi.c
>>         @@ -1392,7 +1392,7 @@ _mesa_LinkProgram(GLhandleARB programObj)
>>            static GLcharARB *
>>            read_shader(const char *fname)
>>            {
>>         -   const int max = 50*1000;
>>         +   int shader_size = 0;
>>               FILE *f = fopen(fname, "r");
>>               GLcharARB *buffer, *shader;
>>               int len;
>>         @@ -1401,8 +1401,16 @@ read_shader(const char *fname)
>>                  return NULL;
>>               }
>>
>>         -   buffer = malloc(max);
>>         -   len = fread(buffer, 1, max, f);
>>         +   /* allocate enough room for the entire shader */
>>         +   fseek(f, 0, SEEK_END);
>>         +   shader_size = ftell(f);
>>         +   rewind(f);
>>         +   assert(shader_size);
>>         +
>>         +   buffer = malloc(shader_size);
>>
>>
>>     Do you have to add one for the terminating zero?
>>
>>
>>
>>         +   assert(buffer);
>>         +
>>         +   len = fread(buffer, 1, shader_size, f);
>>               buffer[len] = 0;
>>
>>               fclose(f);
>>         --
>>
>>
>>     I thought I was the only person who ever used this code!
>>
>>     Other than the one question above this looks alright.
>>
>>     Reviewed-by: Brian Paul <brianp@vmware.com <mailto:brianp@vmware.com
>> >>
>>
>>     _________________________________________________
>>     mesa-dev mailing list
>>     mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org
>> >
>>     http://lists.freedesktop.org/__mailman/listinfo/mesa-dev
>>     <https://urldefense.proofpoint.com/v1/url?u=http:/
>> /lists.freedesktop.org/mailman/listinfo/mesa-dev&k=
>> oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%
>> 2BTLs8wadB%2BiIj9xpBY%3D%0A&m=BQB5tezP6JJarG3K6deQKB%2FgjK%
>> 2FGhKQvd8LRF8usgCs%3D%0A&s=6d587c1f9718375e59390ea001cad3
>> ee836a3597c86c77596f02292c818b3051>
>>
>>
>>
>>
>>
>> --
>>   Cody Northrop
>>   Graphics Software Engineer
>>   LunarG, Inc.- 3D Driver Innovations
>>   Email: cody@lunarg.com <mailto:cody@lunarg.com>
>>   Website: http://www.lunarg.com
>> <https://urldefense.proofpoint.com/v1/url?u=http:/
>> /www.lunarg.com/&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=
>> lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=
>> BQB5tezP6JJarG3K6deQKB%2FgjK%2FGhKQvd8LRF8usgCs%3D%0A&s=
>> e4c25bb2e76d3a5cbff56061766e2be646a3106ae813979beea26ede2680e7ec>
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>