Bug#712269: [tvtime] cppcheck tool discovered same severe errors in the code

triniton adam <[email protected]> Sat, 15 Jun 2013 06:27:58 +0000
Newsgroups gmane.comp.video.tvtime.devel
Message-ID <CAPuGpVwYedrgPcwATgLVPoDeaxggV8-rQ42BEwh6Hj0pMt7rvA__36302.8432162993$1371278019$gmane$org@mail.gmail.com>
--===============2336379941726937563==
Content-Type: multipart/alternative; boundary=089e011619f6f2adc304df2b7640

--089e011619f6f2adc304df2b7640
Content-Type: text/plain; charset=ISO-8859-1

A short list of bug description:
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
01.  src/commands.c:2409: (performance) Variable 'curname' is reassigned a
value before the old one has been used

Description: This will crash tvtime if we use command "tvtime-command
set_input_width 720"

This bug will be fixed if line 2404:

    const char *curname = menu_get_name( cmd->curusermenu );

will be removed.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
02.  src/leetft.c:81: (error) Memory leak: cur
03.  src/tvtime.c:1435: (error) Memory leak: colourbars
04.  src/tvtime.c:1435: (error) Memory leak: saveframe
05.  src/tvtime.c:1435: (error) Memory leak: fadeframe
06.  src/tvtime.c:1435: (error) Memory leak: blueframe
07.  src/tvtime.c:1485: (error) Memory leak: fadeframe
08.  src/tvtime.c:1304: (error) Memory leak: tvtime

Description: alocated memory isn't freed before function return value.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
09.  intl/localealias.c:306 (error) Resource leak: fp
10.  src/tvtimeconf.c:986: (error) Resource leak: fifofd
11.  src/tvtimeconf.c:1185: (error) Resource leak: fifofd

Description: open file isn't closed before function return value
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
12.  src/utils.c:130: (error) Resource leak: temp_dir

Description: open directory isn't closed before funtion return vale.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

13.  plugins/kdetv_greedyh.c:92: (error) Buffer is accessed out of bounds:
Picture
14.  plugins/kdetv_tomsmocomp.c:91: (error) Buffer

Description:

have

#define MAX_PICTURE_HISTORY 10

and then

    TPicture Picture[ 8 ;

    for( i = 0; i < MAX_PICTURE_HISTORY; i++ ) {
        Info.PictureHistory[ i  = &(Picture[ i );
    }

That's two more elements accessed than provided.
The code would be cleaner and guarded from errors like that if we used:

    TPicture Picture[ MAX_PICTURE_HISTORY ;

instead of the hardcoded 8.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

15.  src/station.c:1034: (warning) Dangerous usage of strncat - 3rd
parameter is the maximum number of characters to append
16.  src/station.c:1034: (error) Dangerous usage of 'filename' (strncpy
doesn't always null-terminate it).

My sugestion is to use memmove insted strncpy:

-    strncpy( filename, getenv( "HOME" ), 235 );
-    strncat( filename, "/.tvtime/stationlist.xml", 255 );

+    static const char config_filename[ = "/.tvtime/stationlist.xml";
+    const char *home = getenv( "HOME" );
+    size_t len = strlen( home );
+    if ( len > sizeof( filename ) - sizeof( config_filename ) )
+        fprintf( stderr, "station: station: Could not create new config
file. The file name will be to long\n" );
+    else {
+        memmove( filename, home, len );
+        memmove( filename + len, config_filename, sizeof( config_filename
) );
+    }
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
17.  src/tvtime.c:242: (warning) Assignment of function parameter has no
effect outside the function.
18.  src/tvtime.c:629: (warning) Assignment of function
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
19.  src/tvtime.c:2549: (warning) The buffer 'prevloc' may not be
null-terminated after the call to strncpy().
20.  src/tvtimeconf.c:393: (warning) The buffer 'prevloc'
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
21.  intl/l10nflist.c:433 -> 23. intl/l10nflist.c:417: (error,
inconclusive) Possible null pointer dereference: retval - otherwise it is
      redundant to check it against null.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
22.  intl/localcharset.c:181: (error) Common realloc mistake: 'res_ptr'
nulled but not freed upon failure
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
23. plugins/greedyh.cpp:29: (warning) Member variable
'DScalerFilterGreedyH::GreedyMaxComb' is not initialized in the constructor.
24. plugins/greedyh.cpp:29: (warning) Member variable
'DScalerFilterGreedyH::GreedyMotionThreshold'
25. plugins/greedyh.cpp:29: (warning) Member variable
'DScalerFilterGreedyH::GreedyMotionSense'
26. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::SearchEffort'
27. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::UseStrangeBob'
28. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pMyMemcpy'
29. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::IsOdd'
30. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pWeaveSrc'
31. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pWeaveSrcP'
32. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pWeaveDest'
33. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pCopySrc'
34. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pCopySrcP'
35. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::pCopyDest'
36. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::src_pitch'
37. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::dst_pitch'
38. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::rowsize'
39. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::height'
40. plugins/tomsmocomp.cpp:29: (warning) Member variable
'DScalerFilterTomsMoComp::FldHeight'
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
41. src/commands.c:1944: (style) Statements following return, break,
continue, goto or throw will never be executed.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
42. src/cpuinfo.c:58: (style) Obsolete function 'usleep' called. It is
recommended to use the 'nanosleep' or 'setitimer' function instead.
43. src/tvtime.c:2130: (style) Obsolete function 'usleep' called.
44. src/tvtime.c:2378: (style) Obsolete function 'usleep' called.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
45. src/xcommon.c:1595: (warning) When using 'char' variables in bit
operations, sign extension can generate unexpected results.

Regards,

George

--089e011619f6f2adc304df2b7640
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>A short list of bug description:<br>-----------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
---------------<br>
01.=A0 src/commands.c:2409: (performance) Variable &#39;curname&#39; is rea=
ssigned a value before the old one has been used<br><br></div>Description: =
This will crash tvtime if we use command &quot;tvtime-command set_input_wid=
th 720&quot;<br>
<div><br>This bug will be fixed if line 2404:<br><br>=A0=A0=A0 const char *=
curname =3D menu_get_name( cmd-&gt;curusermenu );<br><br>will be removed.<b=
r>-------------------------------------------------------------------------=
---------------------------------------------------------------------------=
----------------------------------<br>
02.=A0 src/leetft.c:81: (error) Memory leak: cur<br>03.=A0 src/tvtime.c:143=
5: (error) Memory leak: colourbars<br>04.=A0 src/tvtime.c:1435: (error) Mem=
ory leak: saveframe<br>05.=A0 src/tvtime.c:1435: (error) Memory leak: fadef=
rame<br>
06.=A0 src/tvtime.c:1435: (error) Memory leak: blueframe<br>07.=A0 src/tvti=
me.c:1485: (error) Memory leak: fadeframe<br>08.=A0 src/tvtime.c:1304: (err=
or) Memory leak: tvtime<br><br>Description: alocated memory isn&#39;t freed=
 before function return value.<br>
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
--------------------------------<br>09.=A0 intl/localealias.c:306 (error) R=
esource leak: fp<br>
10.=A0 src/tvtimeconf.c:986: (error) Resource leak: fifofd<br>11.=A0 src/tv=
timeconf.c:1185: (error) Resource leak: fifofd<br><br>Description: open fil=
e isn&#39;t closed before function return value<br>------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
--------<br>
12.=A0 src/utils.c:130: (error) Resource leak: temp_dir<br><br>Description:=
 open directory isn&#39;t closed before funtion return vale.<br>-----------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
---------------------<br>
<br>13.=A0 plugins/kdetv_greedyh.c:92: (error) Buffer is accessed out of bo=
unds: Picture<br>14.=A0 plugins/kdetv_tomsmocomp.c:91: (error) Buffer <br><=
br>Description: <br><br>have<br><br>#define MAX_PICTURE_HISTORY 10<br><br>a=
nd then<br>
<br>=A0=A0=A0 TPicture Picture[ 8 ;<br><br>=A0=A0=A0 for( i =3D 0; i &lt; M=
AX_PICTURE_HISTORY; i++ ) {<br>=A0=A0=A0 =A0=A0=A0 Info.PictureHistory[ i=
=A0 =3D &amp;(Picture[ i );<br>=A0=A0=A0 }<br><br>That&#39;s two more eleme=
nts accessed than provided.<br>The code would be cleaner and guarded from e=
rrors like that if we used:<br>
<br>=A0=A0=A0 TPicture Picture[ MAX_PICTURE_HISTORY ;<br><br>instead of the=
 hardcoded 8.<br>----------------------------------------------------------=
---------------------------------------------------------------------------=
-------------------------------------------------<br>
<br>15.=A0 src/station.c:1034: (warning) Dangerous usage of strncat - 3rd p=
arameter is the maximum number of characters to append<br>16.=A0 src/statio=
n.c:1034: (error) Dangerous usage of &#39;filename&#39; (strncpy doesn&#39;=
t always null-terminate it).<br>
<br>My sugestion is to use memmove insted strncpy:<br><br>-=A0=A0=A0 strncp=
y( filename, getenv( &quot;HOME&quot; ), 235 );<br>-=A0=A0=A0 strncat( file=
name, &quot;/.tvtime/stationlist.xml&quot;, 255 );<br><br>+=A0=A0=A0 static=
 const char config_filename[ =3D &quot;/.tvtime/stationlist.xml&quot;;<br>
+=A0=A0=A0 const char *home =3D getenv( &quot;HOME&quot; );<br>+=A0=A0=A0 s=
ize_t len =3D strlen( home );<br>+=A0=A0=A0 if ( len &gt; sizeof( filename =
) - sizeof( config_filename ) )<br>+=A0=A0=A0 =A0=A0=A0 fprintf( stderr, &q=
uot;station: station: Could not create new config file. The file name will =
be to long\n&quot; );<br>
+=A0=A0=A0 else {<br>+=A0=A0=A0 =A0=A0=A0 memmove( filename, home, len );<b=
r>+=A0=A0=A0 =A0=A0=A0 memmove( filename + len, config_filename, sizeof( co=
nfig_filename ) );<br>+=A0=A0=A0 }<br>-------------------------------------=
---------------------------------------------------------------------------=
----------------------------------------------------------------------<br>
17.=A0 src/tvtime.c:242: (warning) Assignment of function parameter has no =
effect outside the function.<br>18.=A0 src/tvtime.c:629: (warning) Assignme=
nt of function<br>---------------------------------------------------------=
---------------------------------------------------------------------------=
--------------------------------------------------<br>
19.=A0 src/tvtime.c:2549: (warning) The buffer &#39;prevloc&#39; may not be=
 null-terminated after the call to strncpy().<br>20.=A0 src/tvtimeconf.c:39=
3: (warning) The buffer &#39;prevloc&#39; <br>-----------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
---<br>
21.=A0 intl/l10nflist.c:433 -&gt; 23. intl/l10nflist.c:417: (error, inconcl=
usive) Possible null pointer dereference: retval - otherwise it is<br>=A0 =
=A0 =A0 redundant to check it against null.<br>----------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
----<br>
22.=A0 intl/localcharset.c:181: (error) Common realloc mistake: &#39;res_pt=
r&#39; nulled but not freed upon failure<br>-------------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
-<br>
23. plugins/greedyh.cpp:29: (warning) Member variable &#39;DScalerFilterGre=
edyH::GreedyMaxComb&#39; is not initialized in the constructor.<br>24. plug=
ins/greedyh.cpp:29: (warning) Member variable &#39;DScalerFilterGreedyH::Gr=
eedyMotionThreshold&#39; <br>
25. plugins/greedyh.cpp:29: (warning) Member variable &#39;DScalerFilterGre=
edyH::GreedyMotionSense&#39;<br>26. plugins/tomsmocomp.cpp:29: (warning) Me=
mber variable &#39;DScalerFilterTomsMoComp::SearchEffort&#39; <br>27. plugi=
ns/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilterTomsMoCom=
p::UseStrangeBob&#39; <br>
28. plugins/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilter=
TomsMoComp::pMyMemcpy&#39; <br>29. plugins/tomsmocomp.cpp:29: (warning) Mem=
ber variable &#39;DScalerFilterTomsMoComp::IsOdd&#39; <br>30. plugins/tomsm=
ocomp.cpp:29: (warning) Member variable &#39;DScalerFilterTomsMoComp::pWeav=
eSrc&#39; <br>
31. plugins/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilter=
TomsMoComp::pWeaveSrcP&#39; <br>32. plugins/tomsmocomp.cpp:29: (warning) Me=
mber variable &#39;DScalerFilterTomsMoComp::pWeaveDest&#39; <br>33. plugins=
/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilterTomsMoComp:=
:pCopySrc&#39; <br>
34. plugins/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilter=
TomsMoComp::pCopySrcP&#39; <br>35. plugins/tomsmocomp.cpp:29: (warning) Mem=
ber variable &#39;DScalerFilterTomsMoComp::pCopyDest&#39; <br>36. plugins/t=
omsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilterTomsMoComp::s=
rc_pitch&#39; <br>
37. plugins/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilter=
TomsMoComp::dst_pitch&#39; <br>38. plugins/tomsmocomp.cpp:29: (warning) Mem=
ber variable &#39;DScalerFilterTomsMoComp::rowsize&#39;<br>39. plugins/toms=
mocomp.cpp:29: (warning) Member variable &#39;DScalerFilterTomsMoComp::heig=
ht&#39;<br>
40. plugins/tomsmocomp.cpp:29: (warning) Member variable &#39;DScalerFilter=
TomsMoComp::FldHeight&#39; <br>--------------------------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------<br>
41. src/commands.c:1944: (style) Statements following return, break, contin=
ue, goto or throw will never be executed.<br>------------------------------=
---------------------------------------------------------------------------=
---------------------------------------------------------------------------=
--<br>
42. src/cpuinfo.c:58: (style) Obsolete function &#39;usleep&#39; called. It=
 is recommended to use the &#39;nanosleep&#39; or &#39;setitimer&#39; funct=
ion instead.<br>43. src/tvtime.c:2130: (style) Obsolete function &#39;uslee=
p&#39; called. <br>
44. src/tvtime.c:2378: (style) Obsolete function &#39;usleep&#39; called.<b=
r>-------------------------------------------------------------------------=
---------------------------------------------------------------------------=
----------------------------------<br>
45. src/xcommon.c:1595: (warning) When using &#39;char&#39; variables in bi=
t operations, sign extension can generate unexpected results.<br><br>Regard=
s,<br><br>George</div></div>

--089e011619f6f2adc304df2b7640--



--===============2336379941726937563==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev
--===============2336379941726937563==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Tvtime-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tvtime-devel

--===============2336379941726937563==--