On Sat, Sep 14, 2013 at 09:11:15PM -0400, Scott Talbert wrote:
> +int _mh_write_config_to_file(uint8_t *in, uint32_t size, char *file_name)
> +{
> + int zip_err;
> + struct zip *zip = zip_open(file_name, ZIP_CREATE | ZIP_EXCL, &zip_err);
> + if (!zip) {
> + if (zip_err == ZIP_ER_EXISTS) {
> + printf("Error: file %s already exists\n", file_name);
> + } else {
> + debug("Failed to create zip file %s", file_name);
Do you want to use `zip_error_to_str` here?
> + }
> + return LC_ERROR_OS_FILE;
> + }
> + int index;
> +
> + // Write XML
> + extern const char *mh_config_header;
> + int xml_buffer_len = strlen(mh_config_header) + 100;
Why 100? Is that just a guess for the extra data your snprintf()ing in? is it
exact, or a guess? can you comment this?
> + char *xml_buffer = new char[xml_buffer_len];
> + uint16_t checksum = mh_get_checksum(in, size);
> + int xml_len = snprintf(xml_buffer, xml_buffer_len, mh_config_header,
> + size, size - 6, checksum, ri.skin);
> + if (xml_len >= xml_buffer_len) {
> + debug("Error, XML buffer length exceeded");
> + return LC_ERROR;
> + }
> + struct zip_source *xml = zip_source_buffer(zip, xml_buffer, xml_len, 0);
why not use freep and let it free this on it's own?
> + if (!xml) {
> + debug("Failed to create zip_source_buffer for XML file");
> + return LC_ERROR_OS_FILE;
> + }
> + index = zip_add(zip, "Description.xml", xml);
According to the docs:
The function zip_add() is the obsolete version of zip_file_add(3).
> + if (index == -1) {
> + debug("Error writing XML to zip file");
> + zip_source_free(xml);
> + return LC_ERROR_OS_FILE;
> + }
> +
> + // Write EzHex file
> + struct zip_source *ezhex = zip_source_buffer(zip, in, size, 0);
> + if (!ezhex) {
> + debug("Failed to create zip_source_buffer for EzHex file");
> + return LC_ERROR_OS_FILE;
> + }
> + index = zip_add(zip, "Result.EzHex", ezhex);
> + if (index == -1) {
> + debug("Error writing EzHex to zip file");
> + zip_source_free(ezhex);
> + return LC_ERROR_OS_FILE;
> + }
You have a leak here... you only ever free 'xml' zip_buffer on error, so in
this case you return and free ezhex, but NOT xml.
> + struct zip *zip = zip_open(file_name, 0, NULL);
> + if (zip) {
The code gets a lot cleaner here if you change this to:
if (!zip) {
// error
}
//code
I know - you were just keeping it as it was, but it was worth pointing out.
> + /* reset the sequence number to 0 */
> + const uint8_t msg_reset_seq[MH_MAX_PACKET_SIZE] =
We do this a lot - perhaps it should be abstracted into a function?
> +// Calculates the XOR checksum for a config read from the remote.
> +uint16_t mh_get_checksum(uint8_t* rd, const uint32_t len)
> +{
> + // This is the "SEED" that all the configs from the website use.
> + uint16_t cksum = 0x4321;
> + // The part of the config that gets checksummed is consistently 6 bytes
> + // less than the length of the config. Since we are checksumming two
> + // bytes at a time, we stop when i == len - 7, which is the same as
> + // i + 1 == len - 6. In the case of odd lengths, we skip the last byte.
> + for (int i = 0; i < (len - 7); i += 2) {
> + uint16_t j = (rd[i+1] << 8) + rd[i];
> + cksum ^= j;
> + }
> + debug("CHECKSUM=0x%04x", cksum);
> + return cksum;
> +}
How did you figure that out?
> + /*
> + * There are four extra bytes at the end of every config file, but they
> + * are not present when read from the remote. Add them here.
> + */
Wait - but mh_find_eof says that you often get those 4 bytes plus a extra
padding?
--
Phil Dibowitz p...@ipom.com
Open Source software and tech docs Insanity Palace of Metallica
http://www.phildev.net/ http://www.ipom.com/
"Be who you are and say what you feel, because those who mind don't matter
and those who matter don't mind."
- Dr. Seuss
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ LIMITED TIME SALE - Full Year of Microsoft Training For Just $49.99! 1,500+ hours of tutorials including VisualStudio 2012, Windows 8, SharePoint 2013, SQL 2012, MVC 4, more. BEST VALUE: New Multi-Library Power Pack includes Mobile, Cloud, Java, and UX Design. Lowest price ever! Ends 9/22/13. http://pubads.g.doubleclick.net/gampad/clk?id=64545871&iu=/4140/ostg.clktrk
_______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
