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

[Roberto Togni]

On Fri, 2 Mar 2007 17:28:54 -0500
"Nicholas Kain" <njkain at gmail.com> wrote:

> On 3/2/07, Nicholas Kain <njkain at gmail.com> wrote:
> > I'll attempt to thread the messages beneath this one, barring any
> > smtp delays.
> 
> Anything but mutt is a pain...
> 
> Patch for the mplayer root directory.

Quick review, only about playtree smil parser

> --- playtreeparser.c.orig	2007-03-02 03:04:29.000000000 -0500
> +++ playtreeparser.c	2007-03-02 03:17:08.000000000 -0500
> @@ -99,8 +99,7 @@ play_tree_parser_get_line(play_tree_pars
>    else
>      return NULL;
>    if(line_end - p->iter > 0)
> -    strncpy(p->line,p->iter,line_end - p->iter);
> -  p->line[line_end - p->iter] = '\0';
> +    strlcpy(p->line,p->iter,line_end - p->iter);
>    if(end[0] != '\0')
>      end++;
>  
> @@ -434,8 +433,8 @@ parse_m3u(play_tree_parser_t* p) {
>  
>  static play_tree_t*
>  parse_smil(play_tree_parser_t* p) {
> -  int entrymode=0;
> -  char* line,source[512],*pos,*s_start,*s_end,*src_line;
> +  int entrymode=0, size;
> +  char* line,*source = NULL,*pos,*s_start,*s_end,*src_line;
>    play_tree_t *list = NULL, *entry = NULL, *last_entry = NULL;
>    int is_rmsmil = 0;
>    unsigned int npkt, ttlpkt;
> @@ -500,8 +499,9 @@ parse_smil(play_tree_parser_t* p) {
>          payload++;
>        // Skip ") at the end of the last line from the current packet
>        line[strlen(line)-2] = 0;
> -      line = realloc(line, strlen(line)+strlen(payload));
> -      strcat (line, payload);
> +      size = strlen(line)+strlen(payload)+1; 
> +      line = realloc(line, size);
> +      strlcat (line, payload, size);

The +1 is a bugfix and will be applied. About the rest it's the same as
in realrtsp: buffer was just created with the right size, why checkit
again while writing into it?

>        npkt++;
>      } else
>        line = strdup(src_line);
> @@ -526,12 +526,9 @@ parse_smil(play_tree_parser_t* p) {
>                mp_msg(MSGT_PLAYTREE,MSGL_V,"Error parsing this source
> line %s\n",line); continue;   
>              }
> -          if (s_end-s_start> 511) {
> -            mp_msg(MSGT_PLAYTREE,MSGL_V,"Cannot store such a large
> source %s\n",line);
> -            continue;
> -          }
> -          strncpy(source,s_start,s_end-s_start);
> -          source[(s_end-s_start)]='\0'; // Null terminate
> +          size = s_end-s_start+1;
> +          source = malloc(size);
> +          strlcpy(source,s_start,size);

Probably ok, but that's an unrelated functional change (remove the
512 bytes line limit). this may be desiderable since i have a sample
with a smil file on a single line (unsupported for other reasons atm).

>            entry = play_tree_new();
>            play_tree_add_file(entry,source);
>            if(!list)  //Insert new entry
> @@ -553,12 +550,9 @@ parse_smil(play_tree_parser_t* p) {
>            mp_msg(MSGT_PLAYTREE,MSGL_V,"Error parsing this source
> line %s\n",line); continue;
>          }
> -        if (s_end-s_start> 511) {
> -          mp_msg(MSGT_PLAYTREE,MSGL_V,"Cannot store such a large
> source %s\n",line);
> -          continue;
> -        }
> -        strncpy(source,s_start,s_end-s_start);
> -        source[(s_end-s_start)]='\0'; // Null terminate
> +        size = s_end-s_start+1;
> +        source = malloc(size);
> +        strlcpy(source,s_start,size);

Same as above

>          entry = play_tree_new();
>          play_tree_add_file(entry,source);
>          if(!list)  //Insert new entry
> @@ -570,8 +564,8 @@ parse_smil(play_tree_parser_t* p) {
>      }
>    }
>  
> -  if (line)
> -    free(line);
> +  free(line);
> +  free(source);
>  
>    if(!list) return NULL; // Nothing found
>  

Ciao,
 Roberto