Releasing IO permissions was done by hand everywhere. Use a proper abstraction. Kill unneeded #include statements. Move #include statements for serprog inside #ifdef.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-cleanup/flash.h =================================================================== --- flashrom-cleanup/flash.h (Revision 671) +++ flashrom-cleanup/flash.h (Arbeitskopie) @@ -308,6 +308,7 @@ struct pci_dev *pci_card_find(uint16_t vendor, uint16_t device, uint16_t card_vendor, uint16_t card_device); void get_io_perms(void); +void release_io_perms(void); int internal_init(void); int internal_shutdown(void); void internal_chip_writeb(uint8_t val, chipaddr addr); Index: flashrom-cleanup/serprog.c =================================================================== --- flashrom-cleanup/serprog.c (Revision 671) +++ flashrom-cleanup/serprog.c (Arbeitskopie) @@ -21,6 +21,13 @@
#include <string.h> #include <stdlib.h> +#include <stdio.h> +#include "flash.h" + +char *serprog_param = NULL; + +#if SERPROG_SUPPORT == 1 + #include <ctype.h> #include <fcntl.h> #include <sys/types.h> @@ -31,16 +38,10 @@ #include <netdb.h> #include <sys/stat.h> #include <errno.h> -#include <stdio.h> #include <unistd.h> #include <inttypes.h> #include <termios.h> -#include "flash.h"
-char *serprog_param = NULL; - -#if SERPROG_SUPPORT == 1 - #define MSGHEADER "serprog:"
#define S_ACK 0x06 Index: flashrom-cleanup/nic3com.c =================================================================== --- flashrom-cleanup/nic3com.c (Revision 671) +++ flashrom-cleanup/nic3com.c (Arbeitskopie) @@ -20,10 +20,7 @@
#include <stdlib.h> #include <string.h> -#include <fcntl.h> #include <sys/types.h> -#include <sys/stat.h> -#include <errno.h> #include "flash.h"
#define BIOS_ROM_ADDR 0x04 @@ -99,9 +96,7 @@
free(pcidev_bdf); pci_cleanup(pacc); -#if defined(__FreeBSD__) || defined(__DragonFly__) - close(io_fd); -#endif + release_io_perms(); return 0; }
Index: flashrom-cleanup/satasii.c =================================================================== --- flashrom-cleanup/satasii.c (Revision 671) +++ flashrom-cleanup/satasii.c (Arbeitskopie) @@ -22,10 +22,6 @@
#include <stdlib.h> #include <string.h> -#include <fcntl.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <errno.h> #include "flash.h"
#define PCI_VENDOR_ID_SII 0x1095 @@ -77,9 +73,7 @@ { free(pcidev_bdf); pci_cleanup(pacc); -#if defined(__FreeBSD__) || defined(__DragonFly__) - close(io_fd); -#endif + release_io_perms(); return 0; }
Index: flashrom-cleanup/internal.c =================================================================== --- flashrom-cleanup/internal.c (Revision 671) +++ flashrom-cleanup/internal.c (Arbeitskopie) @@ -95,6 +95,13 @@ } }
+void release_io_perms(void) +{ +#if defined(__FreeBSD__) || defined(__DragonFly__) + close(io_fd); +#endif +} + int internal_init(void) { int ret = 0; @@ -128,9 +135,7 @@
int internal_shutdown(void) { -#if defined(__FreeBSD__) || defined(__DragonFly__) - close(io_fd); -#endif + release_io_perms();
return 0; }
On 8/9/09 3:47 PM, Carl-Daniel Hailfinger wrote:
Releasing IO permissions was done by hand everywhere. Use a proper abstraction. Kill unneeded #include statements. Move #include statements for serprog inside #ifdef.
I think serproc.c should not be compiled if SERPROG_SUPPORT is 0, instead of commenting out all code in that file.
With that fixed, this is Acked-by: Stefan Reinauer stepan@coresystems.de
On 09.08.2009 15:51, Stefan Reinauer wrote:
On 8/9/09 3:47 PM, Carl-Daniel Hailfinger wrote:
Releasing IO permissions was done by hand everywhere. Use a proper abstraction. Kill unneeded #include statements. Move #include statements for serprog inside #ifdef.
I think serproc.c should not be compiled if SERPROG_SUPPORT is 0, instead of commenting out all code in that file.
With that fixed, this is Acked-by: Stefan Reinauer stepan@coresystems.de
Well, this is the same mechanism we use for ft2232_spi.c and back then it was picked because it allowed us to avoid lots of ifdefs in generic code. We'd need an ifdef in option parsing, an ifdef in the programmer struct, an ifdef in the programmer enum, etc. With all those ifdefs, it gets easy to miss one. Plus, we'd need to use #ifdef for the flashrom usage message, the --list-supported parameter, and possibly the man page. IMHO that's a nightmare and won't get better if we add support for more external programmers. I have two of them (unfinished) on my disk, Uwe has one or two as well. The size benefits of not compiling serprog.c at all are ~256 bytes.
Regards, Carl-Daniel
On 8/9/09 4:03 PM, Carl-Daniel Hailfinger wrote:
Well, this is the same mechanism we use for ft2232_spi.c and back then it was picked because it allowed us to avoid lots of ifdefs in generic code. We'd need an ifdef in option parsing, an ifdef in the programmer struct, an ifdef in the programmer enum, etc. With all those ifdefs, it gets easy to miss one. Plus, we'd need to use #ifdef for the flashrom usage message, the --list-supported parameter
Which would be a clean way of doing it, otherwise we end up with a list of programmers in the code that are not at all supported.
The size benefits of not compiling serprog.c at all are ~256 bytes.
I am not worried about the size but about dead code being compiled into a mission critical program.
Stefan
On 09.08.2009 15:51, Stefan Reinauer wrote:
On 8/9/09 3:47 PM, Carl-Daniel Hailfinger wrote:
Releasing IO permissions was done by hand everywhere. Use a proper abstraction. Kill unneeded #include statements. Move #include statements for serprog inside #ifdef.
I think serproc.c should not be compiled if SERPROG_SUPPORT is 0, instead of commenting out all code in that file.
With that fixed, this is Acked-by: Stefan Reinauer stepan@coresystems.de
Thanks, I committed this (except the serprog part) in r672.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Stefan Reinauer