Le dimanche 08 novembre 2015 à 15:53 +0100, Nico Huber a écrit : > Hi Paul, > > On 24.10.2015 13:19, Paul Kocialkowski wrote: > > I will probably send v2 right away, feel free to follow up the > > discussion on some of these comments in there, it'll probably be > > easier. > I'll keep some of the general discussion here. Some new comments > will come inline with v2. > > >>> + unsigned char buffer[edi_read_buffer_length]; > >>> + unsigned int i; > >>> + int rc; > >>> + > >>> + edi_read_cmd((unsigned char *)cmd, address); > >>> + > >>> + memset(buffer, 0, sizeof(buffer)); > >> Why? > > > > Same thing here, spi_send_command *should* fill-in the whole buffer, > > but in case it doesn't, I'd rather have known values instead, just in > > case garbage turns out to take one of EDI_READY or EDI_NOT_READY. > > > > This is probably not a hard requirement either, but seems like a good > > programming habit to produce reliable code to me. > What bothers me is that you imply that spi_send_command() may behave > wrong. That is very defensive programming. And if spi_send_command() > really does it wrong, we'd never know.
So I guess the point here is that spi_send_command will let us know in its return code when it failed to fill the whole buffer. I can work with that as a hard assumption. Some functions, such as read, read at most the requested number of bytes, thus this programming habit becomes relevant. > Well, that's generally why I prefer to never initialize unless it's > necessary. So many errors out there that we can't see due to over > initialization :-/ At the end of the day, I prefer to have initialized data mistakenly interpreted than random garbage (but of course, that's only a fallback and it indicates that there is a problem somewhere else). I guess the question is whether we want to have a chance to spot those kinds of issues at run time by not initializing data and having better odds of spotting a hard crash or have it keep going with a fallback path. For flashrom, I'd say it's okay to have a hard fail. For other projects, where it is vital for the program to keep running no matter what, I wouldn't agree. -- Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: https://www.replicant.us/ Blog: https://blog.replicant.us/ Wiki/tracker/forums: https://redmine.replicant.us/
signature.asc
Description: This is a digitally signed message part
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
