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 770196 - cppcheck results run against maint branch
cppcheck results run against maint branch
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Backend - XML
git-maint
Other Linux
: Normal normal
: ---
Assigned To: gnucash-core-maint
gnucash-core-maint
Depends on:
Blocks:
 
 
Reported: 2016-08-21 13:06 UTC by pjacquod
Modified: 2018-06-29 23:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to comment #2 (756 bytes, patch)
2016-08-21 13:18 UTC, pjacquod
needs-work Details | Review
reworked patch... (950 bytes, patch)
2016-08-22 19:53 UTC, pjacquod
committed Details | Review

Description pjacquod 2016-08-21 13:06:51 UTC
I run cppcheck against the source code (currently maint) and it showed some errors and warnings.

If you do not mind, I will try to identify them and propose fixes - if I think those are not false positive.  If you are not interested in, just mention it, I won't do it.  

If you are Ok, I will this bug as meta bug and reference for patches against maint branch proposed due to the cppcheck run. 

Regards
Comment 1 pjacquod 2016-08-21 13:18:40 UTC
Created attachment 333818 [details] [review]
patch to comment #2
Comment 2 pjacquod 2016-08-21 13:20:34 UTC
cppcheck error: 

[src/backend/xml/sixtp.c:173]: (error) va_list 'ap' was opened but not closed by va_end().

In 

sixtp*
sixtp_set_any(sixtp *tochange, int cleanup, ...)

va_list is closed in any case, except when tochange is NULL and NULL is directly returned.

patch attached.
Comment 3 John Ralls 2016-08-21 14:47:21 UTC
Comment on attachment 333818 [details] [review]
patch to comment #2

Don't you think it would make more sense to move va_start to after the if (!tochange) block instead?

I'll also point out that the XML backend is next in line for C++ rewrite. This code will continue to be used through the end of the 2.6 cycle but won't be in 2.8.
Comment 4 pjacquod 2016-08-22 19:52:29 UTC
: - ) well patch changed based on your remark. 
I used to program C++, but a bit old, I need to get reflex back. 

For what will be rewrite or not... 
I do not have a lot of free time, but would like to try (and see) if I have enough time to dive a bit a give back something to the project. So with some tools, this will let me look here and there in the code. If time allows (and I am still interested), I will ask / propose for more... (I have SQL know-how and some feature ideas, but let's first go with small steps:- ).

regards
Comment 5 pjacquod 2016-08-22 19:53:08 UTC
Created attachment 333938 [details] [review]
reworked patch...

well reworked with your hint
Comment 6 John Ralls 2016-08-22 20:05:45 UTC
Thanks, I'll commit your patch shortly.

If it's been a few years since you last programmed C++, the language has changed a lot. You'll have some studying to do.

I brought up the C++ rewrite because repairing static analysis errors on code that's going to get a major overhaul RSN is IMO a waste of time and I don't want to waste yours.

Gnome, gnome-utils, and gnome-query are furthest down the list for overhaul, so if you really want to do static analysis cleanup those would be better directories to work in. I frankly think we have higher priorities than static analysis cleanup though. Could I interest you on working on bugs?
Comment 7 Geert Janssens 2016-11-30 15:57:04 UTC
As the patch has been committed I'll be closing this bug.

@pjacquod: thanks and additional patches are still welcome :)

For future contributions you can either open a new bug report or directly open a pull request on github.

While static analysis is not high on the priority list, each contribution is still welcome. As John suggests we have lots of open bugs, many of which probably can be fixed with small changes. So if you find time again in the future, I encourage you to look into those as well.
Comment 8 John Ralls 2018-06-29 23:50:44 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=770196. Please update any external references or bookmarks.