On Sat, Apr 21, 2012 at 11:01 PM, Stefan Schmidt wrote: >> ---------------------------------------------------------------- >> Stefan Schmidt (2): >> dfu_file: Add function to generate and add a DFU suffix to a file > > Thanks for the leak fix on this one. > >> dfu-suffix: New DFU suffix manipulation tool > > >> Tormod Volden (21): >> dfu_file: Verify that we could read the whole file >> dfu_file: Return failure if CRC does not match >> dfu-suffix: Print version before banner, and do it every time > > All above are fine. > >> Port file handling to stdio streams >> Do not overwrite existing files when uploading > > Might be a good idea to squeeze the overwrite fix into the file > handling commit. Its a fix for this commit anyway. Changes the other > things incremental is fine but without this squashed we would have a > regression. Especially one that overrides a user file. :/ Sure > therefore the user would need to compile it with the file IO port > patch but without the fix. Still good practice.
Yes, totally fine with me. I had just kept it in a separate commit because the porting itself was kind of search and replace, and this append/fseek "hack" was the only non-obvious thing. So I have now squashed it and re-pushed, but kept the long explaining commit messages. (Of course, the first commit would not overwrite a user file unless the user tried to overwrite it!) > > The fseek() for sync read/write streams was interesting, btw. > >> dfu-suffix: Check for availability of truncate() function > > You happen to know which platforms are missing this? No, but I would guess everything non-posix. I believe, unless you are using cygwin, the posix layer on Windows is not so complete. I know that when compiling with MinGW, truncate() is not available. I just found this http://stackoverflow.com/questions/584639/truncate-file so it looks like we can use SetEndOfFile() on Windows instead. I will look into that another day. > >> Check for availability of getpagesize() >> Replace usleep() and sleep() by milli_sleep() macro > > Is HAVE_WINDOWS_H really the preferred way to check for windows? Satz > used ENV_WINDOWS (which also looked odd to me). Could be that it > really depends on what you are using to create the executable. MinGW > vs. Windows compiler. Anyway, as you have tested this under Windows > and I also have no problems with it under Linux thats fine. http://msdn.microsoft.com/en-us/library/windows/desktop/ms686298%28v=vs.85%29.aspx says that you should include windows.h to use the Sleep() function. So maybe not the preferred way to check for Windows but seems a good guess for checking for Sleep(). > >> configure.ac: Remove unnecessary tests >> Remove some obsolete, unused code >> Use standard library (C99) types instead of OS types >> Only conditionally include unistd.h >> usb_dfu.h: Pack struct properly for Microsoft compilers >> Various "pedantic" code compliance fixes > > The usage string split into three printfs looks pretty ugly to me. Yes, that seems pretty random, but I estimated it to keep each string below the standard max length, and I kind of grouped options together logically. Another way would be to have printf's on every line... > >> Const'ify some strings and add some explicit casts >> Do not use leading underscores in #include guards >> dfuse: Add progress indication on download > > For consistency I would go with a # here instead of a dot. The > non-DfuSe part of dfu-util uses it for progress indication. Yes, but I did not add the text and delimiters around it either. I see many tools use dots for this kind of progress. For now it makes it easier for me to spot in a log if the right download option is being used, so consistency is not a goal :) I can reconsider this later, maybe we can use dots everywhere. > >> Detect DfuSe device correctly on big-endian architectures >> main: Stop guessing transfer size, error out instead > > I agree, its about time to rely on the functional descriptor. Lets see > if any bug reports will come for this. :) BTW, I have come to realize that we are getting no bug reports, so we have to google for use of dfu-util and proactively discover the users' problems. That's how I found the Maple fall-out! > >> main: Update copyright banner >> Add quirk for Maple since it reports wrong DFU version > > So this is without confirmation from leaflabs, right? Its fine anyway > as it can only improve the situation. :) Yes, they are not responsive. Since they are shipping dfu-util in their product, I expected a little bit more of cooperation from them. > > The squashing of the bug fix in the previous change is the only thing > that would block me from pulling this in. If you could fix this up I > would happily pull in an updated pull request. > Done! > From my side we would be ready for a 0.6 release afterwards. :) We could maybe wait to see if I can get suffix truncation work on Windows easily, that would save some documentation and make dfu-util fully consistent across architectures. Cheers, Tormod _______________________________________________ devel mailing list devel@lists.openmoko.org https://lists.openmoko.org/mailman/listinfo/devel
