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

[Nicholas Kain]

On 3/2/07, Nico Sabbi <nicola_sabbi at fastwebnet.it> wrote:
>  >--- stream/stream_dvb.c.orig    2007-03-02 11:18:16.000000000 -0500
>  >+++ stream/stream_dvb.c 2007-03-02 13:42:36.000000000 -0500
>  >@@ -176,8 +176,7 @@ static dvb_channels_list *dvb_get_channe
>  >                        ptr->name = (char*) malloc(k+1);
>  >                        if(! ptr->name)
>  >                                continue;
>  >-                       strncpy(ptr->name, line, k);
>  >-                       ptr->name[k] = 0;
>  >+                       strlcpy(ptr->name, line, k+1);
>
> it's shorter, but to me it looks equivalent
>
>
>  >                }
>  >                else
>  >                        continue;
>  >@@ -764,7 +763,7 @@ dvb_config_t *dvb_get_config(void)
>  >        conf->cards = NULL;
>  >        for(i=0; i<MAX_CARDS; i++)
>  >        {
>  >-               sprintf(filename, "/dev/dvb/adapter%d/frontend0", i);
>  >+               snprintf(filename, sizeof
>  >filename,"/dev/dvb/adapter%d/frontend0", i);
>  >                fd = open(filename, O_RDONLY|O_NONBLOCK);
>

> [...code snipped...]
>
> filename is allocated as char[30] and MAX_CARDS is defined as 4, so in
> no way sprintf() can overflow
>
> [...code snipped...]
>
> same reasoning here

Yeah, those are identical, and there will be quite a few others that
will compile to equivalent code;  I've converted away from sprintf in
those instances because it aids in searching the codebase for code
that has not yet been checked.  For the same reason, I've annotated
certain uses of strncpy() and strcpy() as being necessary and safe.
All of this is to aid in maintenance and future audits.  Without
changes like these, it is extremely time consuming to check for
correctness.