[AVaRICE-user] Bug in AVaRICE 2.13
Hi, the attached patch fixes an incorrect return statement: Apparently old GCC versions implicitly casted "false" to "NULL", which no longer is the case. So the "return false;" statement in jtagrw.cc:134 in function uchar *jtag1::jtagRead(unsigned long addr, unsigned int numBytes) should be replaced by "return NULL;" Also, there is a lot of inconsistent use of tabs and spaces for indent. Maybe running astyle, uncrustify, or whatever of the code would be a good idea? ;-) Kind regards, Marian - M.Sc. Marian Buschsieweke Dept. Communication and Networked Systems (ComSys) Institute for Intelligent Cooperating Systems (IKS) Otto-von-Guericke-University of Magdeburg Universitätsplatz 2, Building 29, Room 314 39106 Magdeburg Germany http://www.comsys.ovgu.de/Team/Marian+Buschsieweke.html Tel.: +49 - 391 - 67 - 52673 Fax: +49 - 391 - 67 - 41161 --- a/src/jtagrw.cc +++ b/src/jtagrw.cc @@ -131,7 +131,7 @@ else numLocations = (numBytes + 1) / 2; if (numLocations > 256) - return false; + return NULL; command[1] = whichSpace; command[2] = numLocations - 1; pgpTYizF0vdjH.pgp Description: OpenPGP digital signature -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot___ avarice-user mailing list avarice-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/avarice-user
Re: [AVaRICE-user] Bug in AVaRICE 2.13
As Marian Buschsieweke wrote: > the attached patch fixes an incorrect return statement: Apparently old GCC > versions implicitly casted "false" to "NULL", which no longer is the case. So > the "return false;" statement in jtagrw.cc:134 in function > > uchar *jtag1::jtagRead(unsigned long addr, unsigned int numBytes) > > should be replaced by "return NULL;" See bug report #24 (filed just recently) > Also, there is a lot of inconsistent use of tabs and spaces for indent. Maybe > running astyle, uncrustify, or whatever of the code would be a good idea? ;-) Changing whitespace just for the purpose of changing it is generally not a good idea. It potentially obfuscates real code changes later on. That doesn't mean maintaining a consistent style is a bad idea, but I wouldn't want to commit changes just for that. -- cheers, Joerg .-.-. --... ...-- -.. . DL8DTL http://www.sax.de/~joerg/ Never trust an operating system you don't have sources for. ;-) -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ avarice-user mailing list avarice-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/avarice-user
Re: [AVaRICE-user] Bug in AVaRICE 2.13
Dear Jörg Wunsch, thanks for the quick reply. > See bug report #24 (filed just recently) Actually I had a look into the bug tracker and before writing the email and couldn't find it. I also wanted to add a bug, but I really don't want to create an account on SourceForge. Every time I click on something there a huge "we value your privacy" banner is jumping in my face. And I only need to click on "More Options" to see the provide that this is a lie... My point is: SourceForge.net is a platform that really scares me away from an open source project, no matter how cool it is. Other coders and potential contributors may feel the same way about it. Maybe with teahub.io a good alternative will (hopefully) emerge some day in the (hopefully) not too distant future. Maybe that is worth for AVaRICE to look into, when it becomes available? > Changing whitespace just for the purpose of changing it is generally > not a good idea. It potentially obfuscates real code changes later on. (Sorry for writing a quite emotional rant about it, but inconsistent indent is one of my pet peeves:) I cannot disagree more on that. I see a VCS log as a tool to tweak the code - not writing code as a tool to tweak the VCS log. Inconsistent coding style makes reading and understanding the code harder. It increases the barrier to get other people to contribute code and identifying bugs harder. E.g. the CVE-2013-1266 (a.k.a. goto fail) [1] would most likely be detected sooner when correct code indentation would have been used. This even lead to the new "-Wmisleading-indentation" warning option in GCC [2]. So getting code indentation consistent (no matter if you prefer tabs, spaces or smart tabs) has a huge impact on code quality, even if this only changes white space. Kind regards, Marian Buschsieweke [1]: https://nvd.nist.gov/vuln/detail/CVE-2014-1266 [2]: https://developers.redhat.com/blog/2016/02/26/gcc-6-wmisleading-indentation-vs-goto-fail/ - M.Sc. Marian Buschsieweke Dept. Communication and Networked Systems (ComSys) Institute for Intelligent Cooperating Systems (IKS) Otto-von-Guericke-University of Magdeburg Universitätsplatz 2, Building 29, Room 314 39106 Magdeburg Germany http://www.comsys.ovgu.de/Team/Marian+Buschsieweke.html Tel.: +49 - 391 - 67 - 52673 Fax: +49 - 391 - 67 - 41161 - On Tue, 10 Jul 2018 12:21:58 +0200 Joerg Wunsch wrote: > As Marian Buschsieweke wrote: > > > the attached patch fixes an incorrect return statement: Apparently old GCC > > versions implicitly casted "false" to "NULL", which no longer is the case. > > So > > the "return false;" statement in jtagrw.cc:134 in function > > > > uchar *jtag1::jtagRead(unsigned long addr, unsigned int numBytes) > > > > should be replaced by "return NULL;" > > See bug report #24 (filed just recently) > > > Also, there is a lot of inconsistent use of tabs and spaces for indent. > > Maybe > > running astyle, uncrustify, or whatever of the code would be a good idea? > > ;-) > > Changing whitespace just for the purpose of changing it is generally > not a good idea. It potentially obfuscates real code changes later on. > > That doesn't mean maintaining a consistent style is a bad idea, but I > wouldn't want to commit changes just for that. pgpd19aq6Yd20.pgp Description: OpenPGP digital signature -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot___ avarice-user mailing list avarice-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/avarice-user
Re: [AVaRICE-user] Bug in AVaRICE 2.13
As Marian Buschsieweke wrote: > My point is: SourceForge.net is a platform that really scares me away from an > open source project, no matter how cool it is. Other coders and potential > contributors may feel the same way about it. Maybe with teahub.io a good > alternative will (hopefully) emerge some day in the (hopefully) not too > distant > future. Maybe that is worth for AVaRICE to look into, when it becomes > available? Seriously, AVaRICE has bigger problems but its hosting platform or indentation style (or VCS, in case anyone's asking for Git now ...). Sourceforge hasn't been my decision, I merely inherited the project that way, but for just distributing the software, it doesn't matter much anyway which platform it is lying on. So far, there are *zero* active developers, plus a number of things that require a rewrite of larger portions of the code. Namely, the entire handling of the AtmelICE (and JTAGICE-3 in HID mode, as well as all the embedded debuggers of the various Xplained boards) is just alpha quality, and could be considered flakey at best. This is way more important than stylistic things (as important as they might be to you personally), because it affects the core functionality of the tool for the only existing current Atmel debugging hardware. I don't have the time to do anything on it, sorry. I can probably merge the current patch (which is a trivial one), and I could also be willing to just roll out a new release if needed, but that's it. If there's anyone going to jump in, who wants to *seriously* (and long-term) maintain AVaRICE, it would be their decision where to host the project (or just keep it on SF.net). They are then also free to migrate to the VCS they feel most comfortable with, and to fix stylistic issues. -- cheers, Joerg .-.-. --... ...-- -.. . DL8DTL http://www.sax.de/~joerg/ Never trust an operating system you don't have sources for. ;-) -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ avarice-user mailing list avarice-user@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/avarice-user
