[MPlayer-dev-eng] [PATCH 1/4] String handling audit/cleanup take 2

[Nicholas Kain]

On 3/3/07, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> Now, this IMO is an example of a good change. But not of the best
> possible solution because
> 1) output of "remote" content in the end should preferably be filtered,
> I think there are some console escape sequences that can do some really
> non-nice things

Yeah, IMO the ideal approach there would be to filter all such
characters by default in the mp_msg functions, and add an additional
function (or a flag that may be turned on, but defaults to being off)
that will allow control codes to be passed in the rare cases that they
may be needed.

In the attached patch, I've dodged the issue by simply not printing
the invalid value provided.  The element name should be enough, I'd
hope.

> 2) We could just avoid all this string handling problems by splitting
> this mp_msg and call mp_msg instead of sprintf in the loop.

I've done that in the attached patch.  It's just the single chunk
under discussion.
-------------- next part --------------
--- asxparser.c.orig	2007-03-02 02:21:30.000000000 -0500
+++ asxparser.c	2007-03-03 12:03:07.000000000 -0500
@@ -108,27 +108,15 @@ asx_attrib_to_enum(const char* val,char*
 static void
 asx_warning_attrib_invalid(ASX_Parser_t* parser, char* elem, char* attrib,
 			   char** valid_vals,const char* val) {
-  char *str,*vals,**ptr;
-  int len;
+  char **ptr;
 
   if(valid_vals == NULL || valid_vals[0] == NULL) return;
   
-  len = strlen(valid_vals[0]) + 1;
-  for(ptr = valid_vals+1 ; ptr[0] != NULL; ptr++) {
-    len += strlen(ptr[0]);
-    len += ((ptr[1] == NULL) ? 4 : 2);
-  }
-  str = vals = malloc(len);
-  vals += sprintf(vals,"%s",valid_vals[0]);
-  for(ptr = valid_vals + 1 ; ptr[0] != NULL ; ptr++) {
-    if(ptr[1] == NULL)
-      vals += sprintf(vals," or %s",ptr[0]);
-    else
-      vals += sprintf(vals,", %s",ptr[0]);
-  }
-  mp_msg(MSGT_PLAYTREE,MSGL_ERR,"at line %d : attribute %s of element %s is invalid (%s). Valid values are %s",
-	      parser->line,attrib,elem,val,str);
-  free(str);
+  mp_msg(MSGT_PLAYTREE, MSGL_ERR, "at line %d : attribute %s of element %s is invalid. Valid values are: ",
+         parser->line,attrib,elem);
+  for(ptr = valid_vals; ptr[0] != NULL; ptr++)
+    mp_msg(MSGT_PLAYTREE, MSGL_ERR, "%s ", ptr[0]);
+	mp_msg(MSGT_PLAYTREE, MSGL_ERR, "\n");
 }
 
 static int
@@ -190,8 +193,7 @@ asx_parse_attribs(ASX_Parser_t* parser,c
       }
     }
     attrib = malloc(ptr2-ptr1+2);
-    strncpy(attrib,ptr1,ptr2-ptr1+1);
-    attrib[ptr2-ptr1+1] = '\0';
+    strlcpy(attrib,ptr1,ptr2-ptr1+2);
 
     ptr1 = strchr(ptr3,'"');
     if(ptr1 == NULL || ptr1[1] == '\0') ptr1 = strchr(ptr3,'\'');
@@ -208,8 +210,7 @@ asx_parse_attribs(ASX_Parser_t* parser,c
     }
     ptr1++;
     val = malloc(ptr2-ptr1+1);
-    strncpy(val,ptr1,ptr2-ptr1);
-    val[ptr2-ptr1] = '\0';
+    strlcpy(val,ptr1,ptr2-ptr1+1);
     n_attrib++;
     
     attribs = (char**)realloc(attribs,(2*n_attrib+1)*sizeof(char*));
@@ -323,8 +324,7 @@ asx_get_element(ASX_Parser_t* parser,cha
   }
 
   element = malloc(ptr2-ptr1+1);
-  strncpy(element,ptr1,ptr2-ptr1);
-  element[ptr2-ptr1] = '\0';
+  strlcpy(element,ptr1,ptr2-ptr1+1);
 
   for( ; strchr(SPACE,*ptr2) != NULL; ptr2++) { // Skip space
     if(ptr2[0] == '\0') {
@@ -353,8 +353,7 @@ asx_get_element(ASX_Parser_t* parser,cha
   // Save attribs string
   if(ptr3-ptr2 > 0) {
     attribs = malloc(ptr3-ptr2+1);
-    strncpy(attribs,ptr2,ptr3-ptr2);
-    attribs[ptr3-ptr2] = '\0';
+    strlcpy(attribs,ptr2,ptr3-ptr2+1);
   }
   //bs_line = parser->line;
   if(ptr3[0] != '/') { // Not Self closed element
@@ -412,8 +411,7 @@ asx_get_element(ASX_Parser_t* parser,cha
 	  //}
 	  ptr4++;
 	  body = malloc(ptr4-ptr3+1);
-	  strncpy(body,ptr3,ptr4-ptr3);
-	  body[ptr4-ptr3] = '\0';	  
+	  strlcpy(body,ptr3,ptr4-ptr3+1);
 	}
 	break;
       } else {