On 2014-12-17 23:11, Anton Lundin wrote:
+// This is a variant of fletcher16 with 16 bit sum instead of 8bit +// and modulo 256 instead of 255 +static unsigned int hw_ostc3_firmware_checksum(hw_ostc3_firmware_t *firmware)
The comment is wrong. It's modulo 65536 (2^16) vs 65535 (2^16-1) due to the 16bit sum.
+static dc_status_t +hw_ostc3_firmware_readline(FILE *fp, unsigned int addr, unsigned char data[], unsigned int size) +{ + unsigned char ascii[40]; + // 1 byte :, 6 bytes addr, X*2 bytes hex -> X bytes data. + const unsigned line_size = size * 2 + 1 + 6 + 1; + unsigned char faddr_byte[3]; + unsigned int faddr = 0; + if (line_size > sizeof(ascii)) + return DC_STATUS_INVALIDARGS; + if (fread (ascii, sizeof(char), line_size, fp) != line_size) + return DC_STATUS_IO; + if (ascii[0] != ':') + return DC_STATUS_DATAFORMAT; + // Is it a CRLF file? + // Read away that trailing LF + if (ascii[line_size] == '\r') + if (fread (ascii + line_size, sizeof(char), 1, fp) != 1) + return DC_STATUS_DATAFORMAT; + if (array_convert_hex2bin(ascii + 1, 6, faddr_byte, sizeof(faddr_byte)) != 0) { + return DC_STATUS_DATAFORMAT; + } + faddr = array_uint24_be (faddr_byte); + if (faddr != addr) + return DC_STATUS_DATAFORMAT; + if (array_convert_hex2bin (ascii + 1 + 6, size*2, data, size) != 0) + return DC_STATUS_DATAFORMAT; + + return DC_STATUS_SUCCESS; +}
There are two bugs in this code. You read line_size bytes, and then try to access ascii[line_size], which is left uninitialized. The ascii array is also not large enough to store CRLF, resulting in a buffer overflow. I have attached a different implementation, which is based on the existing ihex code. It can handle arbitrary line endings, and not just CRLF or LF. So if you don't mind, I prefer to use this version. Jef