Re: [Mesa-dev] [PATCH 3/3] mesa/main/shaderapi: purely non-functional cleanups, like whitespace errors and cleanups
Am 21.05.2018 um 19:21 schrieb Ian Romanick:
> On 05/10/2018 02:05 AM, Benedikt Schemmer wrote:
>> remove a memset too and yes, this is all functionally identical
>>
>> ---
>> src/mesa/main/shaderapi.c | 40
>> 1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index e8acca4490..1d0ca5374b 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -241,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
>> /* Device drivers may override these to control what kind of instructions
>> * are generated by the GLSL compiler.
>> */
>> - struct gl_shader_compiler_options options;
>> + struct gl_shader_compiler_options options = {};
>> gl_shader_stage sh;
>> int i;
>>
>> - memset(&options, 0, sizeof(options));
>
> This is not functionally the same. The memset will zero padding fields
> added by the compiler, but '= {}' does not.
I did check with
https://godbolt.org/
and at least for clang (which generates a memset for {}) and gcc it is exactly
the same
void test(void) {
struct gl_shader_compiler_options options = {};
memset(&options, 0, sizeof(options));
}
gcc:
// = {}
mov QWORD PTR [rbp-32], 0
mov QWORD PTR [rbp-24], 0
mov QWORD PTR [rbp-16], 0
// memset
lea rax, [rbp-32]
mov edx, 24
mov esi, 0
mov rdi, rax
call memset
clang:
// = {}
mov rdi, rsi
mov qword ptr [rbp - 32], rsi # 8-byte Spill
mov esi, eax
mov qword ptr [rbp - 40], rdx # 8-byte Spill
mov dword ptr [rbp - 44], eax # 4-byte Spill
call memset
//memset
mov rdx, qword ptr [rbp - 32] # 8-byte Reload
mov rdi, rdx
mov esi, dword ptr [rbp - 44] # 4-byte Reload
mov rdx, qword ptr [rbp - 40] # 8-byte Reload
call memset
but your right intel compilers generate something weird:
// = {}
lea rax, QWORD PTR [-32+rbp] #71.47
mov edx, 0 #71.47
mov ecx, 24 #71.47
mov rdi, rax #71.47
mov eax, edx #71.47
and eax, 65535 #71.47
mov ah, al #71.47
mov edx, eax #71.47
shl eax, 16 #71.47
or eax, edx #71.47
mov esi, ecx #71.47
shr rcx, 2 #71.47
rep stosd #71.47
mov ecx, esi #71.47
and ecx, 3 #71.47
rep stosb #71.47
// memset
lea rax, QWORD PTR [-32+rbp] #72.5
mov edx, 0 #72.5
mov ecx, 24 #72.5
mov rdi, rax #72.5
mov esi, edx #72.5
mov rdx, rcx #72.5
call memset #72.5
mov QWORD PTR [-8+rbp], rax #72.5
>
> I'm also not fond of the The '!= 0' and '== NULL' changes. Pretty much
> everywhere in core Mesa uses those patterns.>
> I feel like most of this is just a bunch of spurious changes that will
> just make cherry picking patches to stable irritating later on.
>
>> options.MaxUnrollIterations = 32;
>> options.MaxIfDepth = UINT_MAX;
>>
>> @@ -254,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)
>>
>> ctx->Shader.Flags = _mesa_get_shader_flags();
>>
>> - if (ctx->Shader.Flags != 0)
>> + if (ctx->Shader.Flags)
>>ctx->Const.GenerateTemporaryNames = true;
>>
>> /* Extended for ARB_separate_shader_objects */
>> @@ -771,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>>GLint *params)
>> {
>> struct gl_shader_program *shProg
>> - = _mesa_lookup_shader_program_err(ctx, program,
>> "glGetProgramiv(program)");
>> + = _mesa_lookup_shader_program_err(ctx, program,
>> +"glGetProgramiv(program)");
>>
>> /* Is transform feedback available in this context?
>> */
>> @@ -953,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>>*params = shProg->BinaryRetreivableHint;
>>return;
>> case GL_PROGRAM_BINARY_LENGTH:
>> - if (ctx->Const.NumProgramBinaryFormats == 0) {
>> + if (!ctx->Const.NumProgramBinaryFormats) {
>> *params = 0;
>>} else {
>> _mesa_get_program_binary_length(ctx, shProg, params);
>> @@ -974,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>> "linked)");
>> return;
>>}
>> - if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
>> + if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
>> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no compute "
>> "shaders)");
>> return;
>> @@ -1234,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx, struct
>> gl_shader *sh)
>> } else {
>>if (ctx->_Shader->Flags & GLSL_DUMP) {
>> _mesa_log("GLSL source for %s shader %d:\n",
>> - _mesa_shader_stage_to_string(sh->Stage), sh->Name);
>> + _mesa_shader_stage_to_string(sh->Stage), sh->Name);
>> _mesa_log("%s\n", sh->Source);
>>}
>>
>> @@ -1381,13 +1381,13 @@ link_program(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>>GLuint i;
>>
Re: [Mesa-dev] [PATCH 3/3] mesa/main/shaderapi: purely non-functional cleanups, like whitespace errors and cleanups
On 05/10/2018 02:05 AM, Benedikt Schemmer wrote:
> remove a memset too and yes, this is all functionally identical
>
> ---
> src/mesa/main/shaderapi.c | 40
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index e8acca4490..1d0ca5374b 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -241,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
> /* Device drivers may override these to control what kind of instructions
> * are generated by the GLSL compiler.
> */
> - struct gl_shader_compiler_options options;
> + struct gl_shader_compiler_options options = {};
> gl_shader_stage sh;
> int i;
>
> - memset(&options, 0, sizeof(options));
This is not functionally the same. The memset will zero padding fields
added by the compiler, but '= {}' does not.
I'm also not fond of the The '!= 0' and '== NULL' changes. Pretty much
everywhere in core Mesa uses those patterns.
I feel like most of this is just a bunch of spurious changes that will
just make cherry picking patches to stable irritating later on.
> options.MaxUnrollIterations = 32;
> options.MaxIfDepth = UINT_MAX;
>
> @@ -254,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)
>
> ctx->Shader.Flags = _mesa_get_shader_flags();
>
> - if (ctx->Shader.Flags != 0)
> + if (ctx->Shader.Flags)
>ctx->Const.GenerateTemporaryNames = true;
>
> /* Extended for ARB_separate_shader_objects */
> @@ -771,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint program,
> GLenum pname,
>GLint *params)
> {
> struct gl_shader_program *shProg
> - = _mesa_lookup_shader_program_err(ctx, program,
> "glGetProgramiv(program)");
> + = _mesa_lookup_shader_program_err(ctx, program,
> +"glGetProgramiv(program)");
>
> /* Is transform feedback available in this context?
> */
> @@ -953,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
> GLenum pname,
>*params = shProg->BinaryRetreivableHint;
>return;
> case GL_PROGRAM_BINARY_LENGTH:
> - if (ctx->Const.NumProgramBinaryFormats == 0) {
> + if (!ctx->Const.NumProgramBinaryFormats) {
> *params = 0;
>} else {
> _mesa_get_program_binary_length(ctx, shProg, params);
> @@ -974,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
> GLenum pname,
> "linked)");
> return;
>}
> - if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
> + if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no compute "
> "shaders)");
> return;
> @@ -1234,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx, struct
> gl_shader *sh)
> } else {
>if (ctx->_Shader->Flags & GLSL_DUMP) {
> _mesa_log("GLSL source for %s shader %d:\n",
> - _mesa_shader_stage_to_string(sh->Stage), sh->Name);
> + _mesa_shader_stage_to_string(sh->Stage), sh->Name);
> _mesa_log("%s\n", sh->Source);
>}
>
> @@ -1381,13 +1381,13 @@ link_program(struct gl_context *ctx, struct
> gl_shader_program *shProg,
>GLuint i;
>
>printf("Link %u shaders in program %u: %s\n",
> - shProg->NumShaders, shProg->Name,
> - shProg->data->LinkStatus ? "Success" : "Failed");
> + shProg->NumShaders, shProg->Name,
> + shProg->data->LinkStatus ? "Success" : "Failed");
>
>for (i = 0; i < shProg->NumShaders; i++) {
> printf(" shader %u, stage %u\n",
> - shProg->Shaders[i]->Name,
> - shProg->Shaders[i]->Stage);
> +shProg->Shaders[i]->Name,
> +shProg->Shaders[i]->Stage);
>}
> }
> }
> @@ -1460,7 +1460,7 @@ void
> _mesa_active_program(struct gl_context *ctx, struct gl_shader_program
> *shProg,
>const char *caller)
> {
> - if ((shProg != NULL) && !shProg->data->LinkStatus) {
> + if ((shProg) && !shProg->data->LinkStatus) {
>_mesa_error(ctx, GL_INVALID_OPERATION,
> "%s(program %u not linked)", caller, shProg->Name);
>return;
> @@ -1794,7 +1794,7 @@ void GLAPIENTRY
> _mesa_GetObjectParameterfvARB(GLhandleARB object, GLenum pname,
>GLfloat *params)
> {
> - GLint iparams[1] = {0}; /* XXX is one element enough? */
> + GLint iparams[1] = {}; /* XXX is one element enough? */
> _mesa_GetObjectParameterivARB(object, pname, iparams);
> params[0] = (GLfloat) iparams[0];
> }
> @@ -1912,7 +1912,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj,
> GLsizei count,
>if (!sh)
> return;
>
> - if (string == NULL) {
Re: [Mesa-dev] [PATCH 3/3] mesa/main/shaderapi: purely non-functional cleanups, like whitespace errors and cleanups
!num is totally fine.
Marek
On Thu, May 17, 2018 at 4:45 AM, Benedikt Schemmer wrote:
>
>
> Am 17.05.2018 um 08:59 schrieb Tapani Pälli:
> >
> >
> > On 05/10/2018 12:05 PM, Benedikt Schemmer wrote:
> >> remove a memset too and yes, this is all functionally identical
> >>
> >> ---
> >> src/mesa/main/shaderapi.c | 40 --
> --
> >> 1 file changed, 20 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> >> index e8acca4490..1d0ca5374b 100644
> >> --- a/src/mesa/main/shaderapi.c
> >> +++ b/src/mesa/main/shaderapi.c
> >> @@ -241,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
> >> /* Device drivers may override these to control what kind of
> instructions
> >> * are generated by the GLSL compiler.
> >> */
> >> - struct gl_shader_compiler_options options;
> >> + struct gl_shader_compiler_options options = {};
> >> gl_shader_stage sh;
> >> int i;
> >>
> >> - memset(&options, 0, sizeof(options));
> >> options.MaxUnrollIterations = 32;
> >> options.MaxIfDepth = UINT_MAX;
> >>
> >> @@ -254,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)
> >>
> >> ctx->Shader.Flags = _mesa_get_shader_flags();
> >>
> >> - if (ctx->Shader.Flags != 0)
> >> + if (ctx->Shader.Flags)
> >> ctx->Const.GenerateTemporaryNames = true;
> >>
> >> /* Extended for ARB_separate_shader_objects */
> >> @@ -771,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint
> program, GLenum pname,
> >> GLint *params)
> >> {
> >> struct gl_shader_program *shProg
> >> - = _mesa_lookup_shader_program_err(ctx, program,
> "glGetProgramiv(program)");
> >> + = _mesa_lookup_shader_program_err(ctx, program,
> >> +"glGetProgramiv(program)");
> >>
> >> /* Is transform feedback available in this context?
> >> */
> >> @@ -953,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint
> program, GLenum pname,
> >> *params = shProg->BinaryRetreivableHint;
> >> return;
> >> case GL_PROGRAM_BINARY_LENGTH:
> >> - if (ctx->Const.NumProgramBinaryFormats == 0) {
> >> + if (!ctx->Const.NumProgramBinaryFormats) {
> >
> > Maybe it's just me having some OCD but with these 'Num' constants I find
> it much easier to read '== 0' than '!' (also below with
> NumProgramBinaryFormats and NumSubroutineUniformRemapTable).
> >
> > I don't feel strong about this though so no need to change this.
>
> I dont have strong feelings about this either, I use a script to replace
> these things.
> In my opinion it just helps to see whether these comparisons have meaning.
> >=1 or ==0 don't really.
> If they do I just use a define to make it clear. Otherwise I find the !
> easier to read and understand.
>
> >
> >>*params = 0;
> >> } else {
> >>_mesa_get_program_binary_length(ctx, shProg, params);
> >> @@ -974,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint
> program, GLenum pname,
> >>"linked)");
> >>return;
> >> }
> >> - if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
> >> + if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
> >>_mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no
> compute "
> >>"shaders)");
> >>return;
> >> @@ -1234,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx,
> struct gl_shader *sh)
> >> } else {
> >> if (ctx->_Shader->Flags & GLSL_DUMP) {
> >>_mesa_log("GLSL source for %s shader %d:\n",
> >> - _mesa_shader_stage_to_string(sh->Stage), sh->Name);
> >> + _mesa_shader_stage_to_string(sh->Stage), sh->Name);
> >>_mesa_log("%s\n", sh->Source);
> >> }
> >>
> >> @@ -1381,13 +1381,13 @@ link_program(struct gl_context *ctx, struct
> gl_shader_program *shProg,
> >> GLuint i;
> >>
> >> printf("Link %u shaders in program %u: %s\n",
> >> - shProg->NumShaders, shProg->Name,
> >> - shProg->data->LinkStatus ? "Success" : "Failed");
> >> + shProg->NumShaders, shProg->Name,
> >> + shProg->data->LinkStatus ? "Success" : "Failed");
> >>
> >> for (i = 0; i < shProg->NumShaders; i++) {
> >>printf(" shader %u, stage %u\n",
> >> - shProg->Shaders[i]->Name,
> >> - shProg->Shaders[i]->Stage);
> >> +shProg->Shaders[i]->Name,
> >> +shProg->Shaders[i]->Stage);
> >> }
> >> }
> >> }
> >> @@ -1460,7 +1460,7 @@ void
> >> _mesa_active_program(struct gl_context *ctx, struct gl_shader_program
> *shProg,
> >>const char *caller)
> >> {
> >> - if ((shProg != NULL) && !shProg->data->LinkStatus) {
> >> + if ((shProg) && !shProg->data->LinkStatus) {
> >
> > remove extra pare
Re: [Mesa-dev] [PATCH 3/3] mesa/main/shaderapi: purely non-functional cleanups, like whitespace errors and cleanups
Am 17.05.2018 um 08:59 schrieb Tapani Pälli:
>
>
> On 05/10/2018 12:05 PM, Benedikt Schemmer wrote:
>> remove a memset too and yes, this is all functionally identical
>>
>> ---
>> src/mesa/main/shaderapi.c | 40
>> 1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
>> index e8acca4490..1d0ca5374b 100644
>> --- a/src/mesa/main/shaderapi.c
>> +++ b/src/mesa/main/shaderapi.c
>> @@ -241,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
>> /* Device drivers may override these to control what kind of
>> instructions
>> * are generated by the GLSL compiler.
>> */
>> - struct gl_shader_compiler_options options;
>> + struct gl_shader_compiler_options options = {};
>> gl_shader_stage sh;
>> int i;
>>
>> - memset(&options, 0, sizeof(options));
>> options.MaxUnrollIterations = 32;
>> options.MaxIfDepth = UINT_MAX;
>>
>> @@ -254,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)
>>
>> ctx->Shader.Flags = _mesa_get_shader_flags();
>>
>> - if (ctx->Shader.Flags != 0)
>> + if (ctx->Shader.Flags)
>> ctx->Const.GenerateTemporaryNames = true;
>>
>> /* Extended for ARB_separate_shader_objects */
>> @@ -771,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>> GLint *params)
>> {
>> struct gl_shader_program *shProg
>> - = _mesa_lookup_shader_program_err(ctx, program,
>> "glGetProgramiv(program)");
>> + = _mesa_lookup_shader_program_err(ctx, program,
>> + "glGetProgramiv(program)");
>>
>> /* Is transform feedback available in this context?
>> */
>> @@ -953,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>> *params = shProg->BinaryRetreivableHint;
>> return;
>> case GL_PROGRAM_BINARY_LENGTH:
>> - if (ctx->Const.NumProgramBinaryFormats == 0) {
>> + if (!ctx->Const.NumProgramBinaryFormats) {
>
> Maybe it's just me having some OCD but with these 'Num' constants I find it
> much easier to read '== 0' than '!' (also below with NumProgramBinaryFormats
> and NumSubroutineUniformRemapTable).
>
> I don't feel strong about this though so no need to change this.
I dont have strong feelings about this either, I use a script to replace these
things.
In my opinion it just helps to see whether these comparisons have meaning. >=1
or ==0 don't really.
If they do I just use a define to make it clear. Otherwise I find the ! easier
to read and understand.
>
>> *params = 0;
>> } else {
>> _mesa_get_program_binary_length(ctx, shProg, params);
>> @@ -974,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
>> GLenum pname,
>> "linked)");
>> return;
>> }
>> - if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
>> + if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
>> _mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no compute
>> "
>> "shaders)");
>> return;
>> @@ -1234,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx, struct
>> gl_shader *sh)
>> } else {
>> if (ctx->_Shader->Flags & GLSL_DUMP) {
>> _mesa_log("GLSL source for %s shader %d:\n",
>> - _mesa_shader_stage_to_string(sh->Stage), sh->Name);
>> + _mesa_shader_stage_to_string(sh->Stage), sh->Name);
>> _mesa_log("%s\n", sh->Source);
>> }
>>
>> @@ -1381,13 +1381,13 @@ link_program(struct gl_context *ctx, struct
>> gl_shader_program *shProg,
>> GLuint i;
>>
>> printf("Link %u shaders in program %u: %s\n",
>> - shProg->NumShaders, shProg->Name,
>> - shProg->data->LinkStatus ? "Success" : "Failed");
>> + shProg->NumShaders, shProg->Name,
>> + shProg->data->LinkStatus ? "Success" : "Failed");
>>
>> for (i = 0; i < shProg->NumShaders; i++) {
>> printf(" shader %u, stage %u\n",
>> - shProg->Shaders[i]->Name,
>> - shProg->Shaders[i]->Stage);
>> + shProg->Shaders[i]->Name,
>> + shProg->Shaders[i]->Stage);
>> }
>> }
>> }
>> @@ -1460,7 +1460,7 @@ void
>> _mesa_active_program(struct gl_context *ctx, struct gl_shader_program
>> *shProg,
>> const char *caller)
>> {
>> - if ((shProg != NULL) && !shProg->data->LinkStatus) {
>> + if ((shProg) && !shProg->data->LinkStatus) {
>
> remove extra parenthesis
>
>> _mesa_error(ctx, GL_INVALID_OPERATION,
>> "%s(program %u not linked)", caller, shProg->Name);
>> return;
>> @@ -1794,7 +1794,7 @@ void GLAPIENTRY
>> _mesa_GetObjectParameterfvARB(GLhandleARB object, GLenum pname,
>>
Re: [Mesa-dev] [PATCH 3/3] mesa/main/shaderapi: purely non-functional cleanups, like whitespace errors and cleanups
On 05/10/2018 12:05 PM, Benedikt Schemmer wrote:
remove a memset too and yes, this is all functionally identical
---
src/mesa/main/shaderapi.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index e8acca4490..1d0ca5374b 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -241,11 +241,10 @@ _mesa_init_shader_state(struct gl_context *ctx)
/* Device drivers may override these to control what kind of instructions
* are generated by the GLSL compiler.
*/
- struct gl_shader_compiler_options options;
+ struct gl_shader_compiler_options options = {};
gl_shader_stage sh;
int i;
- memset(&options, 0, sizeof(options));
options.MaxUnrollIterations = 32;
options.MaxIfDepth = UINT_MAX;
@@ -254,7 +253,7 @@ _mesa_init_shader_state(struct gl_context *ctx)
ctx->Shader.Flags = _mesa_get_shader_flags();
- if (ctx->Shader.Flags != 0)
+ if (ctx->Shader.Flags)
ctx->Const.GenerateTemporaryNames = true;
/* Extended for ARB_separate_shader_objects */
@@ -771,7 +770,8 @@ get_programiv(struct gl_context *ctx, GLuint program,
GLenum pname,
GLint *params)
{
struct gl_shader_program *shProg
- = _mesa_lookup_shader_program_err(ctx, program,
"glGetProgramiv(program)");
+ = _mesa_lookup_shader_program_err(ctx, program,
+"glGetProgramiv(program)");
/* Is transform feedback available in this context?
*/
@@ -953,7 +953,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
GLenum pname,
*params = shProg->BinaryRetreivableHint;
return;
case GL_PROGRAM_BINARY_LENGTH:
- if (ctx->Const.NumProgramBinaryFormats == 0) {
+ if (!ctx->Const.NumProgramBinaryFormats) {
Maybe it's just me having some OCD but with these 'Num' constants I find
it much easier to read '== 0' than '!' (also below with
NumProgramBinaryFormats and NumSubroutineUniformRemapTable).
I don't feel strong about this though so no need to change this.
*params = 0;
} else {
_mesa_get_program_binary_length(ctx, shProg, params);
@@ -974,7 +974,7 @@ get_programiv(struct gl_context *ctx, GLuint program,
GLenum pname,
"linked)");
return;
}
- if (shProg->_LinkedShaders[MESA_SHADER_COMPUTE] == NULL) {
+ if (!shProg->_LinkedShaders[MESA_SHADER_COMPUTE]) {
_mesa_error(ctx, GL_INVALID_OPERATION, "glGetProgramiv(no compute "
"shaders)");
return;
@@ -1234,7 +1234,7 @@ _mesa_compile_shader(struct gl_context *ctx, struct
gl_shader *sh)
} else {
if (ctx->_Shader->Flags & GLSL_DUMP) {
_mesa_log("GLSL source for %s shader %d:\n",
- _mesa_shader_stage_to_string(sh->Stage), sh->Name);
+ _mesa_shader_stage_to_string(sh->Stage), sh->Name);
_mesa_log("%s\n", sh->Source);
}
@@ -1381,13 +1381,13 @@ link_program(struct gl_context *ctx, struct
gl_shader_program *shProg,
GLuint i;
printf("Link %u shaders in program %u: %s\n",
- shProg->NumShaders, shProg->Name,
- shProg->data->LinkStatus ? "Success" : "Failed");
+ shProg->NumShaders, shProg->Name,
+ shProg->data->LinkStatus ? "Success" : "Failed");
for (i = 0; i < shProg->NumShaders; i++) {
printf(" shader %u, stage %u\n",
- shProg->Shaders[i]->Name,
- shProg->Shaders[i]->Stage);
+shProg->Shaders[i]->Name,
+shProg->Shaders[i]->Stage);
}
}
}
@@ -1460,7 +1460,7 @@ void
_mesa_active_program(struct gl_context *ctx, struct gl_shader_program *shProg,
const char *caller)
{
- if ((shProg != NULL) && !shProg->data->LinkStatus) {
+ if ((shProg) && !shProg->data->LinkStatus) {
remove extra parenthesis
_mesa_error(ctx, GL_INVALID_OPERATION,
"%s(program %u not linked)", caller, shProg->Name);
return;
@@ -1794,7 +1794,7 @@ void GLAPIENTRY
_mesa_GetObjectParameterfvARB(GLhandleARB object, GLenum pname,
GLfloat *params)
{
- GLint iparams[1] = {0}; /* XXX is one element enough? */
+ GLint iparams[1] = {}; /* XXX is one element enough? */
_mesa_GetObjectParameterivARB(object, pname, iparams);
params[0] = (GLfloat) iparams[0];
}
@@ -1912,7 +1912,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj,
GLsizei count,
if (!sh)
return;
- if (string == NULL) {
+ if (!string) {
_mesa_error(ctx, GL_INVALID_VALUE, "glShaderSourceARB");
return;
}
@@ -1925,7 +1925,7 @@ shader_source(struct gl_context *ctx, GLuint shaderObj,
GLsizei count,
* last element will be set to the to
