Stephen Warren wrote: > Phil Dibowitz wrote: >> Stephen Warren wrote: >>> 1) Ensure all libconcord.h APIs either take in (or send back) a length >>> parameter for all buffers read or created. Most do, a couple don't. >> I thought went through and added lengths anywhere that made sense - did I >> miss places? Could you give me an example? > > The following should be updated: > > (I think I missed the first 3 on my previous round, and the others were > possibly added later.) > > int find_binary_size(uint8_t *ptr, uint32_t *size); > int post_preconfig(uint8_t *data); > int post_postconfig(uint8_t *data);
This is all GetTag() stuff, so it'll depend on how that turns out. > int read_safemode_from_remote(uint8_t **out, lh_callback cb, > void *cb_arg); Ah. OK, I know why it would never occur to me to do this. This is always FIRMWARE_SIZE. We can surely return the size, but the library *ALWAYS* knows the size, so it's rather silly to do this. We'll be passing the user a value for them to pass back that we know already. In fact, if it's not FIRMWARE_SIZE, something's wrong. So I see no need to this. > int write_safemode_to_file(uint8_t *in, char *file_name); > int read_safemode_from_file(char *file_name, uint8_t **out); > int read_firmware_from_remote(uint8_t **out, lh_callback cb, > void *cb_arg); > int write_firmware_to_remote(uint8_t *in, int direct, lh_callback cb, > void *cb_arg); > int write_firmware_to_file(uint8_t *in, char *file_name, int binary); > int read_firmware_from_file(char *file_name, uint8_t **out, int binary); And same will all of these. >>> 2) Pass buffer size into GetTag, so it doesn't assume the input is >>> null-terminated. (There's definitely at least one bug in GetTag right >>> now...) >> Heh - I really hate that code. So, here's a few thoughts I have on this: >> >> - Null-terminated is actually fine if it was *NULL* terminated not 0 >> terminated. I'd like to actually change that to be null-terminated. > > ??? 0-terminated and NULL-terminated are the same thing. Also, don't > confuse NULL termination (0-valued character value) with the NULL > pointer (0 value of pointer type). Doh, I was thinking '0' not 0. ::sigh:: > And incidentally, on a related note, since this is C++ code, 0 and NULL > are defined (by C++ standard) as the same thing to (for pointer values). True... > Well, I guess we could mmap. That might make the code less portable if > the API requires a mmap-style implementation, rather than simply allows > it. That said, this detail can probably be hidden - we'd either have > <SNIP> > If the API did allocate anything internally (e.g. when reading from the > remote) we'd need to make sure there was a separate free_buffer_xxx API > for each case, so they wouldn't get confused. I realize my development experience is almost entirely contained in the UNIX realm, but I was pretty sure mmap() was cross-platform. I know it's POSIX, but I was fairly certain it was available pretty much everywhere. I certainly don't want to handle the file differently on different platforms, that's lame. >>> 4) Add a function to free buffers, so non-C++ clients can effectively >>> call delete[] through the libconcord API for the buffers they got back. >> I actually started on this somewhere based on your comment before but got >> distracted. This definitely needs to happen before the next release. You >> *can* do it, but I think I have a directory somewhere with it done or almost >> done and need to commit it... so feel free leave this one to me if you'd >> like. > > Assuming no allowance for mmap, it's just one function that calls > delete[]. So, I'll probably put it in, and we can augment it if/when > mmap is implemented... I already added this as part of my porting harmony.cpp to harmony.c - see the latest commit. I couldn't remember why I hadn't committed that - it was because I was waiting to finish the port to C... which is now in CVS. My commit in a few minutes will build that by default, too. -- 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
