#69 Coding errors that may be potentially harmful

open-fixed
None
5
2011-05-03
2011-04-27
No

Hi there.

I just installed the version of minidlna from Debian's unstable distribution and I found some coding errors that should be fixed in the near future.

As I wrote on the Debian BTS:

> Unfortunately, the code seems to have some bad coding mistakes that will
> probably manifest themselves in the future as plain bugs and/or security
> issues, so I am reporting this in advance.
>
> Attached is the gzipped output of "cppcheck -v -s ." in the unpacked
> directory of "apt-get source minidlna".
>
> It would be good to let upstream know about these issues.

Regards,

Rogério Brito.

Discussion

  • Justin Maggard

    Justin Maggard - 2011-05-03
    • assigned_to: nobody --> jmaggard
    • status: open --> open-fixed
     
  • Justin Maggard

    Justin Maggard - 2011-05-03

    Thanks for pointing this out. I wasn't aware of the cppcheck tool. The reported errors should all be fixed in CVS now.

     
  • Benoît Knecht

    Benoît Knecht - 2011-06-30

    There are still a few errors reported by 'cppcheck -s -v .' on the latest CVS revision (2011-06-29). I'm just copy-pasting the relevant output, as I don't think I can attach a new file to this bug with the full output.

    [metadata.c:346]: (error) Memory leak: m.mime
    [metadata.c:476]: (error) Memory leak: m.creator
    [metadata.c:476]: (error) Memory leak: m.artist
    [tagutils/tagutils-ogg.c:453]: (error) Resource leak: file
    [tagutils/tagutils-ogg.c:453]: (error) Resource leak: file
    [tivo_beacon.c:182]: (style) Variable 'machine' is assigned a value that is never used
    [tivo_beacon.c:183]: (style) Variable 'platform' is assigned a value that is never used
    [tivo_beacon.c:184]: (style) Variable 'services' is assigned a value that is never used
    [tivo_commands.c:115]: (style) Variable 'ret' is assigned a value that is never used

    --
    Benoît Knecht

     
  • Justin Maggard

    Justin Maggard - 2011-07-01

    Hi Benoit,

    The file handle leak in tagutils-ogg.c is indeed a bug that was not found in older versions of cppcheck. That fix has been checked in. I also checked in fixes for the stylistic complaints in the TiVo area.

    However, the memory leaks detected in metadata.c appear to be false positives from cppcheck. If you can point out how that could leak memory, I'd be more than happy to fix it. But I can't see how a leak is possible there.

     
  • Benoît Knecht

    Benoît Knecht - 2011-07-01

    You're probably right about the memory leaks in metadata.c being false positives, I don't see what the problem could be either.

    Thanks for fixing the other issues.

    --
    Benoît Knecht

     

Log in to post a comment.