Coverity DEADCODE - CID 10719

Bug #944253 reported by Product Strategy Coverity Bug Uploader on 2012-03-01
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
The Sound Menu
Fix Released
Low
Charles Kerr
Fifth
Fix Released
Low
Charles Kerr

Bug Description

This bug is exported from the Coverity Integration Manager on Canonical's servers. For information on how this is done please see this website: https://wiki.ubuntu.com/CanonicalProductStrategy/Coverity
CID: 10719
Checker: DEADCODE
Category: No category available
CWE definition: http://cwe.mitre.org/data/definitions/561.html
File: /tmp/buildd/indicator-sound-0.8.0.0/build/gtk2/src/metadata-menu-item.c
Function: metadata_menuitem_alter_label()
Code snippet:
721 const gchar* _tmp0_;
722 const gchar* _tmp1_;
723 g_return_if_fail (self != NULL);
724 g_return_if_fail (new_title != NULL);
CID 10719 - DEADCODE
After this line (or expression), the value of "_tmp0_" cannot be 0.
725 _tmp0_ = new_title;
CID 10719 - DEADCODE
On this path, the condition "_tmp0_ == NULL" cannot be true.
726 if (_tmp0_ == NULL) {
CID 10719 - DEADCODE
Execution cannot reach this statement "return;".
727 return;
728 }
729 _tmp1_ = new_title;
730 dbusmenu_menuitem_property_set ((DbusmenuMenuitem*) self, DBUSMENU_METADATA_MENUITEM_PLAYER_NAME, _tmp1_);

Related branches

Charles Kerr (charlesk) wrote :

I'm still just learning vala so I could be wrong, but I think Coverity is correct.

The vala code that generated the C that Coverity is warning is:

  public void alter_label (string new_title)
  {
    if (new_title == null) return;
    this.property_set (MENUITEM_PLAYER_NAME, new_title);
  }

I think "new_title" isn't allowed to be null because it's not prefixed with '?' in the argument list.

Removing the "if (new_title == null) return;" line removes the C code that generated the Coverity warning.

Conor Curran (cjcurran) wrote :

Yes I think the signature should be (string? new_title) and then check to see if is NULL. As it stands the check is irrelevant. I would prefer to keep this check in place inorder to protect the UI against rogue dbus updates. Not that i have every witnessed one but it's better off being safe then sorry considering a mplayer's mpris implementation maybe misbehaving (this I have witnessed in the past).

Charles Kerr (charlesk) wrote :

Sounds good to me. Thanks for reviewing, Conor :)

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers