After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740382 - Fixing Cppcheck warnings
Fixing Cppcheck warnings
Status: RESOLVED FIXED
Product: gedit
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Gedit maintainers
Gedit maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-19 18:09 UTC by Boris Egorov
Modified: 2014-11-20 12:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (2.00 KB, patch)
2014-11-19 18:09 UTC, Boris Egorov
needs-work Details | Review
Cppcheck plain text report (1.29 KB, text/plain)
2014-11-19 18:12 UTC, Boris Egorov
  Details
Patch fixing memory leaks (1.41 KB, patch)
2014-11-20 04:59 UTC, Boris Egorov
committed Details | Review
Patch fixing typo (857 bytes, patch)
2014-11-20 04:59 UTC, Boris Egorov
committed Details | Review

Description Boris Egorov 2014-11-19 18:09:20 UTC
Created attachment 291023 [details] [review]
Patch

Cppcheck found a few issues in gedit. Attached patch fixes a typo and a few memory leaks.
Comment 1 Boris Egorov 2014-11-19 18:12:36 UTC
Created attachment 291024 [details]
Cppcheck plain text report

Cppcheck detects more issues, mostly memory leaks in plugins and wrong memory usage. Attach contains a list of remaining issues.
Comment 2 Sébastien Wilmet 2014-11-19 18:47:58 UTC
Review of attachment 291023 [details] [review]:

Thanks. A few comments below, otherwise it looks good.

::: gedit/gedit-preferences-dialog.c
@@ +658,3 @@
 
+	if (!g_file_set_contents (dest_name, contents, length, error)) {
+		g_free (contents);

The code convention in gedit is:
if (blah)
{
}

Normally around one-line blocks too, but some old code doesn't always do that.

::: gedit/gedit-print-preview.c
@@ +568,3 @@
 	priv = preview->priv;
 
+	if (priv->tile_h <= 0 || priv->tile_w <= 0)

Please create a separate commit for this change.
Comment 3 Sébastien Wilmet 2014-11-19 18:51:00 UTC
(In reply to comment #1)
> Cppcheck detects more issues, mostly memory leaks in plugins and wrong memory
> usage. Attach contains a list of remaining issues.

It looks like a very useful tool.
Comment 4 Boris Egorov 2014-11-20 04:58:38 UTC
(In reply to comment #2)
Fixed, make a patchset for these two changes.
Comment 5 Boris Egorov 2014-11-20 04:59:18 UTC
Created attachment 291054 [details] [review]
Patch fixing memory leaks
Comment 6 Boris Egorov 2014-11-20 04:59:51 UTC
Created attachment 291055 [details] [review]
Patch fixing typo
Comment 7 Boris Egorov 2014-11-20 05:08:39 UTC
(In reply to comment #3)
> 
> It looks like a very useful tool.

Yep, it can found many common mistakes. You need to set it flags correctly to have a good result. I run it on gedit like this:

    [gedit root dir] $ cppcheck --library=gtk --force --enable=warning,performance,style --inconclusive --suppressions-list=sup.list 2>errors.log

Suppressions list contains these two lines, for warnings which appear in C projects a lot:

    unusedStructMember
    variableScope

Also, Cppcheck can generate xml and then produce a nice html report. I've set up a regular checking of some free projects with it, including gedit. I'll try to fix more warnings later =)
Comment 8 Sébastien Wilmet 2014-11-20 12:32:09 UTC
Thanks, I've pushed the commits to the master branch (it'll also be available for the 3.14 version).