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

[Nicholas Kain]

> >    ptr=buf;
> >    AV_WB32(ptr, 0xa1e9149d);
> >    ptr+=4;
> > @@ -148,7 +149,7 @@ static void real_calc_response_and_check
> >
> >    /* add tail */
> >    resp_len = strlen (response);
> > -  strcpy (&response[resp_len], "01d0a8e3");
> > +  strlcpy (response + resp_len, "01d0a8e3", CHALLENGE_SIZE -
> > resp_len);
>
> from a quick look resp_len is known, so there is no risk of overflow.

Yeah, it's another functionally equivalent change just to make reviews
easier.  It can be dropped if it's not liked.  There's quite a few like it
as well.  They're generally useful (IMO), since it makes it very quick
to see that attention has been paid to string lengths in the code just
by grepping for 1-2 surrounding lines of context.

> >    /* calculate checksum */
> >    for (i=0; i<resp_len/4; i++)
> > @@ -275,8 +276,9 @@ static rmff_header_t *real_parse_sdp(cha
> >  #ifdef LOG
> >        printf("asmrp rule match: %u for stream %u\n", rulematches[j],
> > desc->stream[i]->stream_id); #endif
> > -      sprintf(b,"stream=%u;rule=%u,", desc->stream[i]->stream_id,
> > rulematches[j]);
> > -      *stream_rules = xbuffer_strcat(*stream_rules, b);
> > +      snprintf(b, sizeof(b), "stream=%u;rule=%u,",
> > desc->stream[i]->stream_id, rulematches[j]);
> > +      *stream_rules = xbuffer_ensure_size(*stream_rules,
> > strlen(*stream_rules)+strlen(b)+1);
> > +      strlcat(*stream_rules, b, strlen(*stream_rules)+strlen(b)+1);
> >      }
>
> Disagree, why remove xbuffer_strcat() ?

I was going to convert the streaming code to natively do things rather
than have a wrapper layer for the original xine buffers, but I doubt
I'll get around to doing that, and if the code is being sync'ed against
the original xine source (I don't know if it is or not), then it would
be undesirable anyway.

These changes can be dropped easily enough if they're
a burden.  Just let me know.