Re: [Gpsbabel-code] clang scan-build report of gpsbabel
Brought to you by:
robertl
From: Thorben T. <r0...@co...> - 2012-05-17 15:58:53
|
Hello, well, you may be right, and it's surely not a bad thing to alert people to potential issues, but also for some part it's a matter of taste to fix all of the minor problems. what i find a little strange is your assumption that people will be willing to devote their time to fixing issues that you just throw at them as an automatically generated report, instead of, say, giving instructions what to fix, or submitting a patch. but then, please just ignore me, i'm not even a developer of gpsbabel, as i said, after spending some time reading your report, i just thought it'd be appropriate to report my opinion on it. it's really something for mr. lipe to decide on, i'll refrain from further comments. - T. On Thu, 17 May 2012 17:33:48 +0200 Bernd Zeimetz <be...@bz...> wrote: > Hi, > > > i'm not even a gpsbabel developer, but i've looked at the report and > > report my findings... > > > >> I've built gpsbabel with clang's scan-build, the current report (which shows 207 > >> bugs) is available at > >> > >> http://devel.recluse.de/scan-build/gpsbabel/ > >> > >> For now I'll rebuild the sources every night and a new report will be generated, > >> assuming there is some interest to fix those bugs :) > > > > sorry, but... > > have you actually read those reports or understand them? > > I did not read all of them, but yes I understand them. Also if you have no idea > what scan-build is you should learn about it, then you will also learn that it > might produce false positives, but in most cases its findings are just right. If > you actually think that there is a real false positive (as in: it is not just > something you ignore for your own lazyness of writing good code), please report > it on the Clang Static Analyzer development mailinglist - see > http://clang-analyzer.llvm.org/ > I'm commenting on some of the things you are mentioning where scan-build is > right on the first look. Don't have the time or the mood to waste my time on > replies to unfriendly mails as yours. > > > > http://devel.recluse.de/scan-build/gpsbabel/2012-05-17-1/report-TxWcEg.html > > your tool does not realize this code is part of a loop, > > and the value will be used on the next iteration? > > The first thing that will be done on the next iteration is > slen = 0; > So the data saved in slen won't be used again. It could even be that slen=0 > sould be moved out of the loop (actually removed as it is initialized already) > to give the loop some more sense. Obviously my "tool" realized that. If can't > know if the first slen=0; in the loop is wrong or if adding something to slen > which is not being used again is wrong, but it shows that there is something > wrong that needs to be fixed. > > > > irrelevant: > > http://devel.recluse.de/scan-build/gpsbabel/2012-05-17-1/report-B4Cv3H.html > > that's an excess initialization of a variable used as a counter. > > it's hardly a bug, and might rather make the code more readable... > > An initialization about 20 lines about the place where exactly the same line > comes up again doesn't make things more readable. Yes, it doesn't do anything > bad, but it is also just not needed to have it in there twice. One day somebody > will add some code in between the two initializations and will have a fun time > trying ti figure out why the introduced code doesn't seem to do anything at all. > > > > and so on... > > Yes and so on. Obviously you are a fan of having dead code in a project. I leave > it to you to learn why dead code / dead stores should be removed. > > > > http://devel.recluse.de/scan-build/gpsbabel/2012-05-17-1/report-hLVZn7.html > > after no less than 30 branches the code might write to an uninitialized > > pointer... this might indeed cause a crash on some malformed input... > > if security was a major issue, an extra check might be justified... > > Security is *always* a major issue. There are people running gpsbabel on data > provided by users on websites, they expect gpsbabel not to choke on malformed > input. Or you just want to convert a "track" you've downloaded from the internet > to have a look at it - you want to trust your favourite tool that using it on > such data is safe. > > > > this one _might_ be valid, the code is too complex to tell right away: > > http://devel.recluse.de/scan-build/gpsbabel/2012-05-17-1/report-ipSdJi.html > > an uninitialized value might be read in some unlikely condition > > when the operation has already failed anyway... > > The code is not complex and the error is well explained. recType is not > initialized if neither tracks nor waypoints were handled before looking on > routes. I don't think that is an unlikely condition, it is a common task to > extract only parts of a given data source. > > > Cheers, > > Bernd > > > -- > Bernd Zeimetz Debian GNU/Linux Developer > http://bzed.de http://www.debian.org > GPG Fingerprint: ECA1 E3F2 8E11 2432 D485 DD95 EB36 171A 6FF9 435F > > ------------------------------------------------------------------------------ > Live Security Virtual Conference > Exclusive live event will cover all the ways today's security and > threat landscape has changed and how IT managers can respond. Discussions > will include endpoint security, mobile security and the latest in malware > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ > _______________________________________________ > Gpsbabel-code mailing list http://www.gpsbabel.org > Gps...@li... > https://lists.sourceforge.net/lists/listinfo/gpsbabel-code |