On Mon, March 3, 2008 2:40 pm, Phil Dibowitz wrote: > I'm not entirely opposed to that. What I *am* opposed to is passing around > sizes that aren't dynamic to users who shouldn't need to know them. > > How would you feel about a new > > struct lh_blob { > uint32_t size; > uint8_t *array; > } > > which we make a pointer to and cast it to a void* and passed back to the > user? This way we can make firmware, safemode fw, and config APIs all > "look" the same?
I guess hiding size in all cases is fine. However, we'd then need to completely remove find_binary_size and find_binary_start from the libharmony API, and have the library do whatever it needs to internally. >> However, that still seems really inconsistent with the other APIs. Also, >> who says firmware blobs *have* to be the exact size of the firmware >> region >> on the device; I see no reason why a device with say 128K of firmware >> space couldn't contain just 119K of firmware, with the rest of the space >> left empty/uninitialized/... And, even if the firmware downloads from >> Logitech currently exactly fill the space, who says they always will; >> shouldn't we be checking file size <= device size, not == device size? > > In theory, that's logical, but a lot of what we do is reverse-engineered > from what the official software does. It's logical a device shouldn't care > about the bits beyond the end of the code, but without some spec, we've > always defaulted to whatever the official software does. > > I'll tell you what - since this seems important to you, do this. Run some > experiments. Try changing the firmware update code to read and write > dynamically. Change a firmware file and nuke the tailing FF bytes, and > then > change the code to be dynamic about the firmware it reads and writes - use > the config code as reference. If this works, leave the code dynamic, and > you > can start passing size in the aforementioned lh_blob struct. I think you mis-understood my point. I meant: It's not written in stone that the EZHex files will always contain precisely FIRMWARE_SIZE bytes of data. You seem to have read: We shouldn't be detecting how much filled data is in the file and only programming that. Let me put what I said another way: The BLOB that we allocate should always contain exactly the number of binary bytes that are present in the EZHex file. We have no idea if the EZHex file will always contain exactly FIRMWARE_SIZE bytes; to be completely robust, we shouldn't fail if it contains less bytes, because they choose not to send unused bytes. If the programming algorithm requires that we always fill the entire FIRMWARE_SIZE area to some specific value, then that detail should be hidden inside the programming routine; it should look at the size of the data buffer it receives, and program that much, then loop to fill up the rest of FIRMWARE_SIZE, if the incoming blob was smaller. > Change a firmware file and nuke the tailing FF bytes, and then > change the code to be dynamic about the firmware it reads and writes > - use the config code as reference. If this works, leave the code > dynamic, and you can start passing size in the aforementioned lh_blob > struct. Given the way flash works, this should actually be fine; the erase sets all bytes to 0xFF, and programming only actually writes zero-bits into the flash device. As such, if *any* byte is 0xFF in the firmware file, then programming it will have no effect on the actual flash content. Now, the software thats running on the remote may care; perhaps it's calculating and validating checksums, or is hard-coded to always program FIRMWARE_SIZE bytes, or has some other dependency on size, or is even buggy. So, I'm certainly not advocating changing what we actually write to the device. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel