From users-return-5223-daniel=haxx.se@subversion.apache.org Thu Oct 7 17:05:15 2010 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on giant.haxx.se X-Spam-Level: X-Spam-Status: No, score=-1.5 required=3.0 tests=BAYES_00,FREEMAIL_FROM, T_RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by giant.haxx.se (8.14.3/8.14.3/Debian-9.1) with SMTP id o97F5D3w030027 for ; Thu, 7 Oct 2010 17:05:14 +0200 Received: (qmail 54502 invoked by uid 500); 7 Oct 2010 15:05:04 -0000 Mailing-List: contact users-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list users@subversion.apache.org Received: (qmail 54495 invoked by uid 99); 7 Oct 2010 15:05:04 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Oct 2010 15:05:04 +0000 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests=FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS Received-SPF: pass (nike.apache.org: local policy) Received: from [217.72.192.221] (HELO fmmailgate01.web.de) (217.72.192.221) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 07 Oct 2010 15:04:56 +0000 Received: from smtp02.web.de ( [172.20.0.184]) by fmmailgate01.web.de (Postfix) with ESMTP id 4B30E16CEA611; Thu, 7 Oct 2010 17:04:36 +0200 (CEST) Received: from [87.123.254.37] (helo=Philipp2) by smtp02.web.de with asmtp (TLSv1:AES128-SHA:128) (WEB.DE 4.110 #24) id 1P3s1M-0000bp-00; Thu, 07 Oct 2010 17:04:36 +0200 Message-ID: From: "Philipp Kloke" To: "Hyrum K. Wright" Cc: References: <91CDEF316E3946318260221A65A49252@Philipp2> In-Reply-To: Subject: Re: Checked Subversions code with cppcheck Date: Thu, 7 Oct 2010 17:04:59 +0200 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_000C_01CB6641.C6479B00" X-Priority: 3 X-MSMail-Priority: Normal Importance: Normal X-Mailer: Microsoft Windows Live Mail 15.4.3502.922 X-MimeOLE: Produced By Microsoft MimeOLE V15.4.3502.922 Sender: philipp.kloke@web.de X-Sender: philipp.kloke@web.de X-Provags-ID: V01U2FsdGVkX19th93bfXHkXnUPcb49mW0PQm/4GCnxbbw9KNe6 GbnHkRWIiVk7ob2CkG0ewLwSqDDyW/aaT0CNuJ06fBVFWIF4KC suYwMd8sL2TlmJrGTU4A== X-Virus-Checked: Checked by ClamAV on apache.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.3.5 (giant.haxx.se [80.67.6.50]); Thu, 07 Oct 2010 17:05:15 +0200 (CEST) X-Friend: Nope Dies ist eine mehrteilige Nachricht im MIME-Format. ------=_NextPart_000_000C_01CB6641.C6479B00 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 8bit ?I am not sure if I am answering correctly (because I usually do not use mailing lists, I just selected the "Answer to all" button of my mail program), but I hope so. I now checked the code again, but with a newer version of cppcheck. The results are in the attachement. If you would like to try to check the code by yourself, see https://sourceforge.net/projects/cppcheck/ (the tool is very easy to use) -----Urspr�ngliche Nachricht----- From: Hyrum K. Wright Sent: Thursday, October 07, 2010 2:56 PM To: Philipp Kloke Cc: users@subversion.apache.org Subject: Re: Checked Subversions code with cppcheck On Mon, Oct 4, 2010 at 9:59 AM, Hyrum K. Wright wrote: > On Mon, Oct 4, 2010 at 9:34 AM, Philipp Kloke > wrote: >> ?Hi, >> >> I used cppcheck to analyze the source code of Subversion. >> Cppcheck claims to have found five errors and some style problems. >> >> Please have a look on the result file in the attachement. > > Thanks for the feedback! I'll look at making some of these changes, > and let you know how it goes. (In the future, as this is related to > the development of Subversion, feel free to send this type of mail > straight to dev@subversion.apache.org.) I've incorporated as many fixes as I could, given the drift on trunk. Could you rerun your tool and report the result back? Thanks, -Hyrum ------=_NextPart_000_000C_01CB6641.C6479B00 Content-Type: text/plain; format=flowed; name="result2.txt"; reply-type=original Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="result2.txt" [subversion\bindings\javahl\native\InputStream.h:45]: (Style) Unused = private function 'InputStream::read' [subversion\bindings\javahl\native\InputStream.h:46]: (Style) Unused = private function 'InputStream::close' [subversion\bindings\javahl\native\JNIThreadData.cpp:40]: (Style) Member variable not initialized in the constructor = 'JNIThreadData::m_formatBuffer' [subversion\bindings\javahl\native\OutputStream.h:44]: (Style) Unused private function 'OutputStream::write' [subversion\bindings\javahl\native\OutputStream.h:46]: (Style) Unused private function 'OutputStream::close' [subversion\bindings\javahl\native\StatusCallback.cpp:37]: (Style) = Member variable not initialized in the constructor 'StatusCallback::wc_ctx' [subversion\bindings\javahl\native\SVNClient.cpp:112] -> [subversion\bindings\javahl\native\SVNClient.h:193]: (Style) The = function 'SVNClient::getLastPath' can be const [subversion\libsvn_auth_kwallet\kwallet.cpp:203]: (Style) Variable 'app' = is assigned a value that is never used [subversion\libsvn_auth_kwallet\kwallet.cpp:273]: (Style) Variable 'app' = is assigned a value that is never used [subversion\libsvn_delta\compose_delta.c:46] -> [subversion\libsvn_delta\compose_delta.c:45]: (Style) Struct 'range_index_node_t' hides typedef with same name [subversion\libsvn_delta\compose_delta.c:70] -> [subversion\libsvn_delta\compose_delta.c:69]: (Style) Struct 'range_list_node_t' hides typedef with same name [subversion\libsvn_delta\compose_delta.c:93] -> [subversion\libsvn_delta\compose_delta.c:92]: (Style) Union = 'alloc_block_t' hides typedef with same name [subversion\libsvn_diff\diff.h:62] -> = [subversion\libsvn_diff\diff.h:38]: (Style) Struct 'svn_diff__position_t' hides typedef with same name [subversion\libsvn_diff\diff.h:69] -> = [subversion\libsvn_diff\diff.h:39]: (Style) Struct 'svn_diff__lcs_t' hides typedef with same name [subversion\libsvn_diff\diff.c:43]: (Style) Variable 'diff' is not = assigned a value [subversion\libsvn_diff\lcs.c:43] -> [subversion\libsvn_diff\lcs.c:41]: (Style) Struct 'svn_diff__snake_t' hides typedef with same name [subversion\libsvn_diff\token.c:42] -> = [subversion\libsvn_diff\diff.h:36]: (Style) Struct 'svn_diff__node_t' hides typedef with same name [subversion\libsvn_diff\token.c:52] -> = [subversion\libsvn_diff\diff.h:37]: (Style) Struct 'svn_diff__tree_t' hides typedef with same name [subversion\libsvn_diff\token.c:144]: (Style) Variable 'start_position' = is not assigned a value [subversion\libsvn_fs_base\dag.c:64] -> [subversion\libsvn_fs_base\dag.h:71]: (Style) Struct 'dag_node_t' hides typedef with same name [subversion\libsvn_fs_base\bdb\env.c:86] -> [subversion\libsvn_fs_base\bdb\env.h:47]: (Style) Struct 'bdb_env_t' = hides typedef with same name [subversion\libsvn_fs_base\bdb\env.c:237]: (Error) Memory leak: = error_info [subversion\libsvn_fs_fs\dag.c:45] -> = [subversion\libsvn_fs_fs\dag.h:62]: (Style) Struct 'dag_node_t' hides typedef with same name [subversion\libsvn_ra_neon\session.c:454]: (Style) Variable 'http_auth_types' is assigned a value that is never used [subversion\libsvn_ra_serf\ra_serf.h:138] -> [subversion\libsvn_ra_serf\ra_serf.h:67]: (Style) Struct 'svn_ra_serf__session_t' hides typedef with same name [subversion\libsvn_ra_serf\ra_serf.h:1485] -> [subversion\libsvn_ra_serf\ra_serf.h:68]: (Style) Struct 'svn_ra_serf__auth_protocol_t' hides typedef with same name [subversion\libsvn_ra_serf\ra_serf.h:596] -> [subversion\libsvn_ra_serf\ra_serf.h:560]: (Style) Struct 'svn_ra_serf__xml_parser_t' hides typedef with same name [subversion\libsvn_ra_serf\commit.c:755]: (Error) Uninitialized = variable: prop_name [subversion\libsvn_ra_serf\merge.c:91] -> [subversion\libsvn_ra_serf\ra_serf.h:1068]: (Style) Struct 'svn_ra_serf__merge_context_t' hides typedef with same name [subversion\libsvn_ra_serf\options.c:65] -> [subversion\libsvn_ra_serf\ra_serf.h:1100]: (Style) Struct 'svn_ra_serf__options_context_t' hides typedef with same name [subversion\libsvn_ra_serf\property.c:65] -> [subversion\libsvn_ra_serf\ra_serf.h:906]: (Style) Struct 'svn_ra_serf__propfind_context_t' hides typedef with same name [subversion\libsvn_ra_serf\replay.c:64] -> [subversion\libsvn_ra_serf\replay.c:62]: (Style) Struct 'replay_info_t' hides typedef with same name [subversion\libsvn_ra_serf\update.c:283] -> [subversion\libsvn_ra_serf\update.c:78]: (Style) Struct = 'report_context_t' hides typedef with same name [subversion\libsvn_ra_svn\ra_svn.h:92] -> [subversion\libsvn_ra_svn\ra_svn.h:64]: (Style) Struct 'svn_ra_svn__session_baton_t' hides typedef with same name [subversion\libsvn_subr\config.c:43] -> [subversion\libsvn_subr\config.c:42]: (Style) Struct 'cfg_section_t' = hides typedef with same name [subversion\libsvn_subr\config.c:58] -> [subversion\libsvn_subr\config.c:57]: (Style) Struct 'cfg_option_t' = hides typedef with same name [subversion\libsvn_subr\error.c:74]: (Style) Redundant assignment of = "err" to itself [subversion\libsvn_subr\opt.c:682]: (Style) Non-empty string test can be simplified to "*peg_rev !=3D '\0'" [subversion\libsvn_subr\win32_xlate.c:74] -> [subversion\libsvn_subr\win32_xlate.h:30]: (Style) Struct = 'win32_xlate_t' hides typedef with same name [subversion\libsvn_wc\adm_ops.c:1202]: (Error) Uninitialized variable: base_status [subversion\libsvn_wc\tree_conflicts.c:407]: (Style) Non-empty string = test can be simplified to "*victim_basename !=3D '\0'" [subversion\libsvn_wc\wc_db_private.h:36] -> [subversion\libsvn_wc\wc_db.h:125]: (Style) Struct 'svn_wc__db_t' hides typedef with same name [subversion\libsvn_wc\wc_db.c:7364]: (Style) The scope of the variable i = can be reduced Warning: It can be unsafe to fix this message. Be careful. Especially = when there are inner loops. Here is an example where cppcheck will write that the scope for 'i' can = be reduced: void f(int x) { int i =3D 0; if (x) { // it's safe to move 'int i =3D 0' here for (int n =3D 0; n < 10; ++n) { // it is possible but not safe to move 'int i =3D 0' here do_something(&i); } } } When you see this message it is always safe to reduce the variable scope = 1 level. [subversion\mod_dav_svn\repos.c:1640] -> [subversion\mod_dav_svn\repos.c:1482]: (Style) Struct 'accept_rec' hides typedef with same name [subversion\mod_dav_svn\version.c:342]: (Style) Variable 'res' is = assigned a value that is never used [subversion\svnserve\main.c:311]: (Style) struct or union member 'serve_thread_t::conn' is never used [subversion\svnserve\main.c:312]: (Style) struct or union member 'serve_thread_t::params' is never used [subversion\svnserve\main.c:313]: (Style) struct or union member 'serve_thread_t::pool' is never used [subversion\svnserve\serve.c:1960]: (Error) Possible null pointer dereference: revprop_items - otherwise it is redundant to check if revprop_items is null at line 1962 [subversion\tests\libsvn_fs_base\fs-base-test.c:1155]: (Style) The scope = of the variable i can be reduced Warning: It can be unsafe to fix this message. Be careful. Especially = when there are inner loops. Here is an example where cppcheck will write that the scope for 'i' can = be reduced: void f(int x) { int i =3D 0; if (x) { // it's safe to move 'int i =3D 0' here for (int n =3D 0; n < 10; ++n) { // it is possible but not safe to move 'int i =3D 0' here do_something(&i); } } } When you see this message it is always safe to reduce the variable scope = 1 level. [subversion\tests\libsvn_fs_base\fs-base-test.c:1089]: (Style) struct or union member 'node_created_rev_args::path' is never used [subversion\tests\libsvn_fs_base\fs-base-test.c:1090]: (Style) struct or union member 'node_created_rev_args::rev' is never used ------=_NextPart_000_000C_01CB6641.C6479B00--