If parsing a program fails, don't change the vertex/fragment program object.

Specifically, don't attach a dummy program.
This change also fixes an occasional segfault.
Some code clean-ups.  Use GLboolean instead of GLuint to return pass/fail.

4 files changed, 52 insertions, 54 deletions

diff --git a/src/mesa/shader/arbfragparse.c b/src/mesa/shader/arbfragparse.c
index 51aa3426ac4..3ef6f8b137a 100644
--- a/src/mesa/shader/arbfragparse.c
+++ b/src/mesa/shader/arbfragparse.c
@@ -182,31 +182,26 @@ _mesa_debug_fp_inst(GLint num, struct fp_instruction *fp)
    }
 }
 
+
 void
 _mesa_parse_arb_fragment_program(GLcontext * ctx, GLenum target,
                                  const GLubyte * str, GLsizei len,
                                  struct fragment_program *program)
 {
-   GLuint a, retval;
+   GLuint i;
    struct arb_program ap;
    (void) target;
 
    /* set the program target before parsing */
    ap.Base.Target = GL_FRAGMENT_PROGRAM_ARB;
 
-   retval = _mesa_parse_arb_program(ctx, str, len, &ap);
-
-   /* XXX: Parse error. Cleanup things and return */
-   if (retval)
-   {
-      program->Instructions = (struct fp_instruction *) _mesa_malloc (
-                                     sizeof(struct fp_instruction) );
-      program->Instructions[0].Opcode = FP_OPCODE_END;
+   if (!_mesa_parse_arb_program(ctx, str, len, &ap)) {
+      /* Error in the program. Just return. */
       return;
    }
 
-   /* copy the relvant contents of the arb_program struct into the
-    * fragment_program struct
+   /* Copy the relevant contents of the arb_program struct into the
+    * fragment_program struct.
     */
    program->Base.String          = ap.Base.String;
    program->Base.NumInstructions = ap.Base.NumInstructions;
@@ -217,8 +212,8 @@ _mesa_parse_arb_fragment_program(GLcontext * ctx, GLenum target,
 
    program->InputsRead     = ap.InputsRead;
    program->OutputsWritten = ap.OutputsWritten;
-   for (a=0; a<MAX_TEXTURE_IMAGE_UNITS; a++)
-      program->TexturesUsed[a] = ap.TexturesUsed[a];
+   for (i = 0; i < MAX_TEXTURE_IMAGE_UNITS; i++)
+      program->TexturesUsed[i] = ap.TexturesUsed[i];
    program->NumAluInstructions = ap.NumAluInstructions;
    program->NumTexInstructions = ap.NumTexInstructions;
    program->NumTexIndirections = ap.NumTexIndirections;
diff --git a/src/mesa/shader/arbprogparse.c b/src/mesa/shader/arbprogparse.c
index 244c85738f5..14912b9321a 100644
--- a/src/mesa/shader/arbprogparse.c
+++ b/src/mesa/shader/arbprogparse.c
@@ -3702,11 +3702,11 @@ debug_variables (GLcontext * ctx, struct var_cache *vc_head,
 /**
  * The main loop for parsing a fragment or vertex program
  *
- * \return 0 on sucess, 1 on error
+ * \return GL_TRUE on success, GL_FALSE on error.
  */
-static GLint
-parse_arb_program (GLcontext * ctx, GLubyte * inst, struct var_cache **vc_head,
-                   struct arb_program *Program)
+static GLboolean
+parse_arb_program(GLcontext * ctx, GLubyte * inst, struct var_cache **vc_head,
+                  struct arb_program *Program)
 {
    GLint err = 0;
 
@@ -3913,10 +3913,10 @@ static int enable_ext (GLcontext *ctx, grammar id, const byte *name, const byte
  * \param ctx - The GL Context
  * \param str - The program string
  * \param len - The program string length
- * \param Program - The arb_program struct to return all the parsed info in
- * \return 0 on sucess, 1 on error
+ * \param program - The arb_program struct to return all the parsed info in
+ * \return GL_TRUE on sucess, GL_FALSE on error
  */
-GLuint
+GLboolean
 _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
                          struct arb_program * program)
 {
@@ -3949,22 +3949,21 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
          _mesa_set_program_error (ctx, error_pos, error_msg);
          _mesa_error (ctx, GL_INVALID_OPERATION,
                       "Error loading grammar rule set");
-         return 1;
+         return GL_FALSE;
       }
 
       err = grammar_check (grammar_syn_id, (byte *) arb_grammar_text, &parsed, &parsed_len);
 
-      /* NOTE: we cant destroy grammar_syn_id right here because grammar_destroy() can
-         reset the last error
-      */
-
+      /* NOTE: we can't destroy grammar_syn_id right here because
+       * grammar_destroy() can reset the last error
+       */
       if (err == 0) {
          grammar_get_last_error ((byte *) error_msg, 300, &error_pos);
          _mesa_set_program_error (ctx, error_pos, error_msg);
          _mesa_error (ctx, GL_INVALID_OPERATION, "Error loading grammar rule set");
 
          grammar_destroy (grammar_syn_id);
-         return 1;
+         return GL_FALSE;
       }
 
       grammar_destroy (grammar_syn_id);
@@ -3979,14 +3978,14 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
       _mesa_set_program_error (ctx, error_pos, error_msg);
       _mesa_error (ctx, GL_INVALID_OPERATION,
                    "Error loading grammer rule set");
-      return 1;
+      return GL_FALSE;
    }
 
    /* Set program_target register value */
    if (set_reg8 (ctx, arbprogram_syn_id, (byte *) "program_target",
       program->Base.Target == GL_FRAGMENT_PROGRAM_ARB ? 0x10 : 0x20)) {
       grammar_destroy (arbprogram_syn_id);
-      return 1;
+      return GL_FALSE;
    }
 
    /* Enable all active extensions */
@@ -4015,7 +4014,7 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
        enable_ext (ctx, arbprogram_syn_id,
           (byte *) "draw_buffers", (byte *) "GL_ARB_draw_buffers")) {
       grammar_destroy (arbprogram_syn_id);
-      return 1;
+      return GL_FALSE;
    }
 
    /* check for NULL character occurences */
@@ -4027,13 +4026,16 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
             _mesa_error (ctx, GL_INVALID_OPERATION, "Lexical Error");
 
             grammar_destroy (arbprogram_syn_id);
-            return 1;
+            return GL_FALSE;
          }
    }
 
    /* copy the program string to a null-terminated string */
-   /* XXX should I check for NULL from malloc()? */
    strz = (GLubyte *) _mesa_malloc (len + 1);
+   if (!strz) {
+      _mesa_error(ctx, GL_OUT_OF_MEMORY, "glprogramStringARB");
+      return GL_FALSE;
+   }
    _mesa_memcpy (strz, str, len);
    strz[len] = '\0';
 
@@ -4048,22 +4050,22 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
       _mesa_free (strz);
       grammar_get_last_error ((GLubyte *) error_msg, 300, &error_pos);
       _mesa_set_program_error (ctx, error_pos, error_msg);
-      _mesa_error (ctx, GL_INVALID_OPERATION, "glProgramStringARB(syntax error)");
+      _mesa_error (ctx, GL_INVALID_OPERATION, "glprogramStringARB(syntax error)");
 
       /* useful for debugging */
 #if DEBUG_PARSING
       do {
          int line, col;
          char *s;
-         fprintf(stderr, "Program: %s\n", (char *) strz);
-         fprintf(stderr, "Error Pos: %d\n", ctx->Program.ErrorPos);
-         s = (char *) _mesa_find_line_column(strz, strz+ctx->Program.ErrorPos, &line, &col);
+         fprintf(stderr, "program: %s\n", (char *) strz);
+         fprintf(stderr, "Error Pos: %d\n", ctx->program.ErrorPos);
+         s = (char *) _mesa_find_line_column(strz, strz+ctx->program.ErrorPos, &line, &col);
          fprintf(stderr, "line %d col %d: %s\n", line, col, s);
       } while (0)
 #endif
 
       grammar_destroy (arbprogram_syn_id);
-      return 1;
+      return GL_FALSE;
    }
 
 #if DEBUG_PARSING
@@ -4095,7 +4097,7 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
    program->VPInstructions = NULL;
 
    vc_head = NULL;
-   err = 0;
+   err = GL_FALSE;
 
    /* Start examining the tokens in the array */
    inst = parsed;
@@ -4103,8 +4105,9 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
    /* Check the grammer rev */
    if (*inst++ != REVISION) {
       _mesa_set_program_error (ctx, 0, "Grammar version mismatch");
-      _mesa_error (ctx, GL_INVALID_OPERATION, "glProgramStringARB(Grammar verison mismatch)");
-      err = 1;
+      _mesa_error(ctx, GL_INVALID_OPERATION,
+                  "glProgramStringARB(Grammar verison mismatch)");
+      err = GL_TRUE;
    }
    else {
       /* ignore program target */
@@ -4125,5 +4128,6 @@ _mesa_parse_arb_program (GLcontext * ctx, const GLubyte * str, GLsizei len,
 #if DEBUG_PARSING
    fprintf (stderr, "_mesa_parse_arb_program() done\n");
 #endif
-   return err;
+
+   return !err;
 }
diff --git a/src/mesa/shader/arbprogparse.h b/src/mesa/shader/arbprogparse.h
index 52db0440447..ececfafa8fd 100644
--- a/src/mesa/shader/arbprogparse.h
+++ b/src/mesa/shader/arbprogparse.h
@@ -67,8 +67,9 @@ struct arb_program
    GLuint NumTexIndirections;
 };
 
-extern GLuint 
+
+extern GLboolean
 _mesa_parse_arb_program( GLcontext *ctx, const GLubyte *str, GLsizei len, 
-                         struct arb_program *Program );
-                          
+                         struct arb_program *program );
+
 #endif
diff --git a/src/mesa/shader/arbvertparse.c b/src/mesa/shader/arbvertparse.c
index 92e3bb352e0..0208389c355 100644
--- a/src/mesa/shader/arbvertparse.c
+++ b/src/mesa/shader/arbvertparse.c
@@ -157,31 +157,29 @@ void _mesa_debug_vp_inst(GLint num, struct vp_instruction *vp)
 }
 
 
+/**
+ * Parse the vertex program string.  If success, update the given
+ * vertex_program object with the new program.  Else, leave the vertex_program
+ * object unchanged.
+ */
 void
 _mesa_parse_arb_vertex_program(GLcontext * ctx, GLenum target,
 			       const GLubyte * str, GLsizei len,
 			       struct vertex_program *program)
 {
-   GLuint retval;
    struct arb_program ap;
    (void) target;
 
    /* set the program target before parsing */
    ap.Base.Target = GL_VERTEX_PROGRAM_ARB;
 
-   retval = _mesa_parse_arb_program(ctx, str, len, &ap);
-
-   /*  Parse error. Allocate a dummy program and return */	
-   if (retval)
-   {
-      program->Instructions = (struct vp_instruction *)
-         _mesa_malloc ( sizeof(struct vp_instruction) );
-      program->Instructions[0].Opcode = VP_OPCODE_END;
+   if (!_mesa_parse_arb_program(ctx, str, len, &ap)) {
+      /* Error in the program. Just return. */
       return;
    }
 
-   /* copy the relvant contents of the arb_program struct into the 
-    * fragment_program struct
+   /* Copy the relevant contents of the arb_program struct into the 
+    * vertex_program struct.
     */
    program->Base.String          = ap.Base.String;
    program->Base.NumInstructions = ap.Base.NumInstructions;