libftdi Archives

Subject: Re: [PATCH 3/6] ftdi_eeprom: Added config value "eeprom_type"

From: Thomas Jarosch <thomas.jarosch@xxxxxxxxxxxxx>
To: libftdi@xxxxxxxxxxxxxxxxxxxxxxx
Cc: Anders Larsen <al@xxxxxxxxxxx>
Date: Fri, 13 Apr 2012 15:01:44 +0200

Hi Anders,


thanks for your patches! I've applied most of them

except 3/6 and 5/6. Below is my comment on 3/6.


On Monday, 9. April 2012 17:29:21 Anders Larsen wrote:

> eeprom_type 0x56 and 0x66 hold 256 bytes, all other hold 128 bytes.


So we still have eeproms of 128 bytes size.


> @@ -323,9 +324,9 @@ int main(int argc, char *argv[])

> {

> if (filename != NULL && strlen(filename) > 0)

> {

> - eeprom_buf = malloc(my_eeprom_size);

> + eeprom_buf = malloc(256);

> FILE *fp = fopen(filename, "rb");

> - fread(eeprom_buf, 1, my_eeprom_size, fp);

> + my_eeprom_size = fread(eeprom_buf, 1, 256, fp);

> fclose(fp);

>

> ftdi_set_eeprom_buf(ftdi, eeprom_buf, my_eeprom_size);


Here you hardcode "256" twice into the code. Any chance you can turn this into a define or "const int"?


Also we should check for fread() errors if we

are going to touch this piece of code :)


Best regards,

Thomas




libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx


Current Thread