[dfu-util] New dfu-suffix manipulation tool

[Patryk Benderz]

[cut]
Hi,
I didn't compile this, just looked on logical consistency with DFU
specification I was able to find [1]. If this is not proper one, please
correct me. 

First what I have noticed is different bcdDFU field. You have hard-coded
0x0100(=0100h, right?), while [1] document is labeled as DFU_1.1.pdf. So
the title of file mentions next minor version number. However in
appendix B (page 41) it is written:
"The bcdDFU field is a two-byte specification revision number. The value
as of this revision of the specification is 0100h, representing version
1.0."
so the document [1] seems to be inconsistent with itself. Shouldn't this
be clarified with usb.org , before hard coding bcdDFU?

> http://cgit.openezx.org/dfu-util/commit/?h=dfu-suffix&id=15757085aa10294b9a58719a42e922fe30405ff8
1.I am not skilled programmer, but I was taught that constant values
like bcdDFU or bLength, in code should be in #define declaration... if
it makes any difference, apart from good practices.

2.I could not find ucDfuSignature field, or similar one anywhere.

3.Order of filling dfusuffix[] matrix is different from what Appendix B
says. This indeed might indicate that you have used different version of
DFU documentation than 1.1. If that is true, do you consider adapting
your patch to fit 1.1 DFU version?

4. And again, do not take it personal, but I was taught to avoid 'goto'
directive, as compilers do not like it. Additionally this kind of
directives are not elegant. So maybe instead using "rewind:" label, it
could be written this way (intentionally left a lot of new lines to ease
reading)"

+ret = lseek(file->fd, 0, SEEK_END);
+if (ret < 0)
+{
+  fprintf(stderr, "Could not seek to DFU suffix\n");
+  perror(file->name);
+}
+else
+{
+  /* After seeking to the end of the file add the suffix */
+  ret = write(file->fd, dfusuffix, sizeof(dfusuffix));
+  if (ret < 0)
+  {
+    fprintf(stderr, "Could not read DFU suffix\n");
+    perror(file->name);
+  } 
+  /* Inserting "ret >=0 && " to avoid entering statement when ret<0 */
+  else if (ret >=0 && ret < sizeof(dfusuffix))
+  {
+    fprintf(stderr, "Could not read whole DFU suffix\n");
+    ret = -EIO;
+  }
+}
+
+lseek(file->fd, 0, SEEK_SET);
+return ret;
+}

I hope I didn't make mistakes.

> release. If you, or anyone else, has some fixes around let me know.
Just a final question about version of DFU. Did you used 1.0 or 1.1?
Will try to look at another commit later.

[1] http://www.google.com/url?q=http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf&sa=U&ei=UkBPT-ivLoKd0QXOz-TjCw&ved=0CAQQFjAA&client=internal-uds-cse&usg=AFQjCNFHUYhSddqeEROv0PVDWXPCYUC8zA

-- 
Patryk "LeadMan" Benderz
Linux Registered User #377521
()  ascii ribbon campaign - against html e-mail
/\  www.asciiribbon.org   - against proprietary attachments