Stephen Warren wrote: > On Mon, March 3, 2008 12:56 am, Phil Dibowitz wrote: >> Stephen Warren wrote: >>> Well, it is true that internally the library knows this. However, I >>> don't see a reason the application shouldn't be "allowed" to know this >>> information too; perhaps the application wants to copy the data or save >>> into some application specific buffer. For this reason, I think we >>> should expose length here too (and check it ==FIRMWARE_SIZE when passed >>> back in for safety.) >> I'm not sure I agree. We already provide a way of saving this data: >> write_firmware_to_file() and write_safemode_to_file(). That array is >> simply >> an opaque data object to the library. I want to be able to change it >> without >> breaking the API, and if possible, the ABI. The user shouldn't touch it. >> Heck, they don't even need to know it's an array - it's just a pointer to >> them. > > Well, in that case, it should be a "void *" not a "uint8_t *", to indicate > explicitly that it's opaque.
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?
As for that "size" piece, I think it's _wrong_ to be passing around sizes
(even hidden like this) on any code that's fundamentally non-dynamic. So it
should just be 0 for static code. More on this below.
> 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.
--
Phil Dibowitz [EMAIL PROTECTED]
Open Source software and tech docs Insanity Palace of Metallica
http://www.phildev.net/ http://www.ipom.com/
"Never write it in C if you can do it in 'awk';
Never do it in 'awk' if 'sed' can handle it;
Never use 'sed' when 'tr' can do the job;
Never invoke 'tr' when 'cat' is sufficient;
Avoid using 'cat' whenever possible" -- Taylor's Laws of Programming
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------- 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
