{"thread":{"id":"19454","subject":"y.a. static code analysis","startedAt":"2009-05-23T19:00:58Z","lastAt":"2009-05-24T13:25:15Z","messageCount":4,"participants":["Serhat Şevki Dinçer","Alex Riesen","Clemens Buchacher"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"114523","messageId":"927245250905231200ifbda2f6t1c54628e314d63e6@mail.gmail.com","threadId":"19454","inReplyTo":null,"subject":"y.a. static code analysis","fromName":"Serhat Şevki Dinçer","fromEmail":"jfcgauss@gmail.com","sentAt":"2009-05-23T19:00:58Z","receivedAt":"2009-05-23T19:00:58Z","isPatch":false,"sender":{"key":"jfcgauss@gmail.com","avatar":null},"body":"hi,\nalthough static code analysis have apparently been used/mentioned\nhere, i did not see any mention of cppcheck\n(http://cppcheck.wiki.sourceforge.net) in the mailist archive. i was\nplaying with cppcheck (1.32) on some OSS, so i decided to try it on\ngit (1.6.3.1) as well.\n$ cppcheck -a -q -s . &> ccgit.txt\npossibly the most useful parts of the output are:\n$ grep -v 'is never used\\|The scope of the variable\\| Error: In' ccgit.txt\ni think only the ones about date.c (below note) are real defects\n(first chars are not checked).\n\nand also how about http://scan.coverity.com? i see it was mentined\nbefore (http://article.gmane.org/gmane.comp.version-control.git/111562)\nwith apparently no responses or arguments (there has been a suggestion\nof bad license terms in that message, but if the scan is suitable for\nso many FOSS (see all rungs) including the kernel, why would it be not\ngood for git?). i think it could be a good free (as in beer) code\ncheck for git.\nregards\n\nnote:\n[./builtin-apply.c:482]: (error) Using 'name' after it is deallocated / released\n[./compat/mingw.c:273]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n[./compat/mkdtemp.c:5]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n[./date.c:268]: (style) Redundant code: Found a statement that begins\nwith numeric constant\n[./date.c:483]: (style) Redundant code: Found a statement that begins\nwith numeric constant\n[./http-push.c:1419]: (error) Using 'lock' after it is deallocated / released\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:729]:\n(all) Array index out of bounds\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:731]:\n(all) Array index out of bounds\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:736]:\n(all) Array index out of bounds\n[./test-sha1.c:16]: (error) Memory leak: buffer\n\n\n[./alloc.c:41]: (style) struct or union member 'any_object::object' is never used\n[./alloc.c:42]: (style) struct or union member 'any_object::blob' is never used\n[./alloc.c:43]: (style) struct or union member 'any_object::tree' is never used\n[./alloc.c:44]: (style) struct or union member 'any_object::commit' is never used\n[./alloc.c:45]: (style) struct or union member 'any_object::tag' is never used\n[./archive-zip.c:31]: (style) struct or union member 'zip_local_header::_end' is never used\n[./archive-zip.c:52]: (style) struct or union member 'zip_dir_header::_end' is never used\n[./archive-zip.c:64]: (style) struct or union member 'zip_dir_trailer::_end' is never used\n[./archive.c:278]: (style) The scope of the variable i can be limited\n[./builtin-apply.c:1445]: (style) The scope of the variable i can be limited\n[./builtin-apply.c:1912]: (style) The scope of the variable added can be limited\n[./builtin-apply.c:2108]: (style) The scope of the variable len can be limited\n[./builtin-apply.c:3246]: (style) The scope of the variable binary can be limited\n[./builtin-apply.c:157]: (style) struct or union member 'patch::deflate_origlen' is never used\n[./builtin-apply.c:482]: (error) Using 'name' after it is deallocated / released\n[./builtin-blame.c:1540]: (style) The scope of the variable time_len can be limited\n[./builtin-blame.c:1541]: (style) The scope of the variable tz can be limited\n[./builtin-checkout.c:599]: (style) The scope of the variable has_dash_dash can be limited\n[./builtin-commit.c:419]: (style) The scope of the variable saved_color_setting can be limited\n[./builtin-grep.c:399]: (style) The scope of the variable kept can be limited\n[./builtin-mailinfo.c:683]: (style) The scope of the variable i can be limited\n[./builtin-merge.c:527]: (style) The scope of the variable ret can be limited\n[./builtin-mv.c:99]: (style) The scope of the variable src_is_dir can be limited\n[./builtin-pack-objects.c:1023]: (style) The scope of the variable used_0 can be limited\n[./builtin-pack-objects.c:1025]: (style) The scope of the variable ofs can be limited\n[./builtin-pack-objects.c:1026]: (style) The scope of the variable c can be limited\n[./builtin-remote.c:484]: (style) The scope of the variable flag can be limited\n[./builtin-remote.c:1002]: (style) The scope of the variable i can be limited\n[./builtin-remote.c:1067]: (style) The scope of the variable i can be limited\n[./builtin-revert.c:52]: (style) The scope of the variable noop can be limited\n[./builtin-show-branch.c:356]: (style) The scope of the variable i can be limited\n[./builtin-show-branch.c:834]: (style) The scope of the variable j can be limited\n[./combine-diff.c:913]: (style) The scope of the variable offset can be limited\n[./compat/mingw.c:273]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n[./compat/mkdtemp.c:5]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n### Error: Invalid number of character {\n[./config.c:747]: (style) The scope of the variable section_len can be limited\n[./config.c:816]: (style) The scope of the variable i can be limited\n[./connect.c:376]: (style) The scope of the variable hostlen can be limited\n[./date.c:370]: (style) The scope of the variable now can be limited\n[./date.c:268]: (style) Redundant code: Found a statement that begins with numeric constant\n[./date.c:483]: (style) Redundant code: Found a statement that begins with numeric constant\n[./diff-no-index.c:209]: (style) The scope of the variable j can be limited\n[./diffcore-break.c:170]: (style) The scope of the variable score can be limited\n[./dir.c:300]: (style) The scope of the variable i can be limited\n[./entry.c:97]: (style) The scope of the variable fd can be limited\n[./entry.c:97]: (style) The scope of the variable ret can be limited\n[./entry.c:100]: (style) The scope of the variable size can be limited\n[./entry.c:101]: (style) The scope of the variable wrote can be limited\n[./fast-import.c:945]: (style) The scope of the variable i can be limited\n[./git.c:138]: (style) The scope of the variable count can be limited\n[./git.c:138]: (style) The scope of the variable option_count can be limited\n[./grep.c:375]: (style) The scope of the variable match can be limited\n[./help.c:132]: (style) The scope of the variable n can be limited\n[./http-push.c:1162]: (style) The scope of the variable sha_ctx can be limited\n[./http-push.c:128]: (style) struct or union member 'transfer_request::errorstr' is never used\n[./http-push.c:1419]: (error) Using 'lock' after it is deallocated / released\n[./imap-send.c:419]: (style) The scope of the variable va can be limited\n[./imap-send.c:431]: (style) The scope of the variable va can be limited\n[./imap-send.c:544]: (style) The scope of the variable n can be limited\n[./imap-send.c:987]: (style) The scope of the variable s can be limited\n[./imap-send.c:988]: (style) The scope of the variable pid can be limited\n[./imap-send.c:1162]: (style) The scope of the variable j can be limited\n[./imap-send.c:1163]: (style) The scope of the variable start can be limited\n[./imap-send.c:33]: (style) struct or union member 'store_conf::path' is never used\n[./imap-send.c:34]: (style) struct or union member 'store_conf::map_inbox' is never used\n[./imap-send.c:42]: (style) struct or union member 'string_list::string' is never used\n[./imap-send.c:48]: (style) struct or union member 'channel_conf::master' is never used\n[./imap-send.c:48]: (style) struct or union member 'channel_conf::slave' is never used\n[./imap-send.c:49]: (style) struct or union member 'channel_conf::master_name' is never used\n[./imap-send.c:49]: (style) struct or union member 'channel_conf::slave_name' is never used\n[./imap-send.c:50]: (style) struct or union member 'channel_conf::sync_state' is never used\n[./imap-send.c:51]: (style) struct or union member 'channel_conf::patterns' is never used\n[./imap-send.c:52]: (style) struct or union member 'channel_conf::mops' is never used\n[./imap-send.c:52]: (style) struct or union member 'channel_conf::sops' is never used\n[./imap-send.c:59]: (style) struct or union member 'group_conf::channels' is never used\n[./imap-send.c:71]: (style) struct or union member 'message::uid' is never used\n[./imap-send.c:72]: (style) struct or union member 'message::status' is never used\n[./imap-send.c:80]: (style) struct or union member 'store::path' is never used\n[./imap-send.c:83]: (style) struct or union member 'store::opts' is never used\n[./imap-send.c:143]: (style) struct or union member 'imap_store_conf::server' is never used\n[./index-pack.c:298]: (style) The scope of the variable base_offset can be limited\n[./object.c:189]: (style) The scope of the variable eaten can be limited\n[./pretty.c:593]: (style) The scope of the variable h1 can be limited\n[./pretty.c:593]: (style) The scope of the variable h2 can be limited\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:729]: (all) Array index out of bounds\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:731]: (all) Array index out of bounds\n[./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:736]: (all) Array index out of bounds\n[./refs.c:69]: (style) The scope of the variable psize can be limited\n[./refs.c:69]: (style) The scope of the variable qsize can be limited\n[./refs.c:69]: (style) The scope of the variable cmp can be limited\n[./sha1_file.c:1926]: (style) The scope of the variable offset can be limited\n[./sha1_file.c:1989]: (style) The scope of the variable size can be limited\n[./sha1_file.c:2519]: (style) The scope of the variable fd can be limited\n[./sha1_name.c:684]: (style) The scope of the variable size can be limited\n[./symlinks.c:76]: (style) The scope of the variable previous_slash can be limited\n[./symlinks.c:77]: (style) The scope of the variable match_flags can be limited\n[./test-sha1.c:16]: (error) Memory leak: buffer\n[./unpack-trees.c:577]: (style) The scope of the variable ret can be limited\n[./upload-pack.c:106]: (style) The scope of the variable i can be limited\n[./xdiff/xprepare.c:140]: (style) The scope of the variable hav can be limited\n"},{"id":"114554","messageId":"81b0412b0905240604q3c8c798bi6ad64d8916b4cd9e@mail.gmail.com","threadId":"19454","inReplyTo":"927245250905231200ifbda2f6t1c54628e314d63e6@mail.gmail.com","subject":"Re: y.a. static code analysis","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-24T13:04:06Z","receivedAt":"2009-05-24T13:04:06Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/5/23 Serhat Şevki Dinçer <jfcgauss@gmail.com>:\n> i think only the ones about date.c (below note) are real defects\n> (first chars are not checked).\n>\n> and also how about http://scan.coverity.com? i see it was mentined\n> before (http://article.gmane.org/gmane.comp.version-control.git/111562)\n> with apparently no responses or arguments (there has been a suggestion\n> of bad license terms in that message, but if the scan is suitable for\n> so many FOSS (see all rungs) including the kernel, why would it be not\n> good for git?). i think it could be a good free (as in beer) code\n> check for git.\n\nThere is a reason why the static checking tools are not popular:\ntoo many false positives.\n\n> [./builtin-apply.c:482]: (error) Using 'name' after it is deallocated / released\n\nJust wrong.\n\n> [./compat/mingw.c:273]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n> [./compat/mkdtemp.c:5]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n\nAssuming the platform (see \"compat\"?) has mkstemp(3).\n\n> [./date.c:268]: (style) Redundant code: Found a statement that begins\n> with numeric constant\n> [./date.c:483]: (style) Redundant code: Found a statement that begins\n> with numeric constant\n\nThere is no numeric constant in the line, and while you're right (almost)\nregarding skipping the first character, the message itself is confusing.\n\nYou're not completely right, because looking at the code, the character\nyou think is skipped is already tested for existence in other places.\nYes, the code could be clearer at this point. Could be just a sign of\nrefactoring passes, though.\n\n> [./http-push.c:1419]: (error) Using 'lock' after it is deallocated / released\n\nThis is the only real bug.\n\n> [./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:729]:\n> (all) Array index out of bounds\n> [./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:731]:\n> (all) Array index out of bounds\n> [./read-cache.c:938] -> [./read-cache.c:759] -> [./read-cache.c:736]:\n> (all) Array index out of bounds\n\nDefinitely not. It is just a flexarray, worked around  with array[1]\nfor some compilers.\n\n> [./test-sha1.c:16]: (error) Memory leak: buffer\n\nThe program ends and there is no point deallocating the buffer.\n"},{"id":"114555","messageId":"20090524131648.GA22800@blimp.localdomain","threadId":"19454","inReplyTo":"81b0412b0905240604q3c8c798bi6ad64d8916b4cd9e@mail.gmail.com","subject":"Re: y.a. static code analysis","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-05-24T13:16:49Z","receivedAt":"2009-05-24T13:16:49Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Noticed and reported by Serhat Şevki Dinçer.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nAlex Riesen, Sun, May 24, 2009 15:04:06 +0200:\n> 2009/5/23 Serhat Şevki Dinçer <jfcgauss@gmail.com>:\n> \n> > [./http-push.c:1419]: (error) Using 'lock' after it is deallocated / released\n> \n> This is the only real bug.\n> \n\nHere's a fix.\n\n http-push.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex dac2c6e..45e8a69 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1415,8 +1415,9 @@ static void remove_locks(void)\n \n \tfprintf(stderr, \"Removing remote locks...\\n\");\n \twhile (lock) {\n+\t\tstruct remote_lock *next = lock->next;\n \t\tunlock_remote(lock);\n-\t\tlock = lock->next;\n+\t\tlock = next;\n \t}\n }\n \n-- \n1.6.3.1.93.g316b2\n"},{"id":"114556","messageId":"20090524132515.GA13030@localhost","threadId":"19454","inReplyTo":"20090524131648.GA22800@blimp.localdomain","subject":"Re: y.a. static code analysis","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-05-24T13:25:15Z","receivedAt":"2009-05-24T13:25:15Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sun, May 24, 2009 at 03:16:49PM +0200, Alex Riesen wrote:\n> Noticed and reported by Serhat Şevki Dinçer.\n> \n> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n\nThanks. Ack.\n"}]}