Vladimir Serbinenko sent a patch for standalone flashrom, and that patch made a few #include statements conditional. While that may still be needed, it is always a good idea to remove unneeded #include statements completely.
unistd.h was only used to get a definition of NULL in all files. Add our own NULL #define and remove unistd.h from flash.h stdio.h has no place in flash.h, it should be included only in files which really need it. Replace a few printf with msg_* to eliminate the need for stdio.h. Add #include statements in individual .c files where needed.
It might make sense to make the system #include statements in some .c files conditional on the circumstances where they are actually needed, especially in files which have OS dependent accesses. Such a patch would be a good followup to this one.
Compile tests on non-x86 and non-Linux platforms highly appreciated.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-include_cleanup/flash.h =================================================================== --- flashrom-include_cleanup/flash.h (Revision 1017) +++ flashrom-include_cleanup/flash.h (Arbeitskopie) @@ -24,9 +24,7 @@ #ifndef __FLASH_H__ #define __FLASH_H__ 1
-#include <unistd.h> #include <stdint.h> -#include <stdio.h> #include "hwaccess.h" #ifdef _WIN32 #include <windows.h> @@ -34,6 +32,10 @@ #undef max #endif
+#ifndef NULL +#define NULL ((void *) 0) +#endif + typedef unsigned long chipaddr;
enum programmer { @@ -556,7 +558,6 @@ extern int verbose; extern const char *flashrom_version; extern char *chip_to_probe; -#define printf_debug(x...) { if (verbose) printf(x); } void map_flash_registers(struct flashchip *flash); int read_memmapped(struct flashchip *flash, uint8_t *buf, int start, int len); int erase_flash(struct flashchip *flash); Index: flashrom-include_cleanup/drkaiser.c =================================================================== --- flashrom-include_cleanup/drkaiser.c (Revision 1017) +++ flashrom-include_cleanup/drkaiser.c (Arbeitskopie) @@ -19,8 +19,6 @@ */
#include <stdlib.h> -#include <string.h> -#include <sys/types.h> #include "flash.h"
#define PCI_VENDOR_ID_DRKAISER 0x1803 Index: flashrom-include_cleanup/nicrealtek.c =================================================================== --- flashrom-include_cleanup/nicrealtek.c (Revision 1017) +++ flashrom-include_cleanup/nicrealtek.c (Arbeitskopie) @@ -21,8 +21,6 @@ #if defined(__i386__) || defined(__x86_64__)
#include <stdlib.h> -#include <string.h> -#include <sys/types.h> #include "flash.h"
#define PCI_VENDOR_ID_REALTEK 0x10ec Index: flashrom-include_cleanup/physmap.c =================================================================== --- flashrom-include_cleanup/physmap.c (Revision 1017) +++ flashrom-include_cleanup/physmap.c (Arbeitskopie) @@ -20,6 +20,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <unistd.h> +#include <stdio.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> @@ -311,7 +313,7 @@ { char msrfilename[64]; memset(msrfilename, 0, 64); - sprintf(msrfilename, "/dev/cpu/%d/msr", cpu); + snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu/%d/msr", cpu);
if (fd_msr != -1) { msg_pinfo("MSR was already initialized\n"); @@ -394,7 +396,7 @@ { char msrfilename[64]; memset(msrfilename, 0, 64); - sprintf(msrfilename, "/dev/cpu%d", cpu); + snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu%d", cpu);
if (fd_msr != -1) { msg_pinfo("MSR was already initialized\n"); Index: flashrom-include_cleanup/sst49lfxxxc.c =================================================================== --- flashrom-include_cleanup/sst49lfxxxc.c (Revision 1017) +++ flashrom-include_cleanup/sst49lfxxxc.c (Arbeitskopie) @@ -20,7 +20,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-#include <stdlib.h> #include "flash.h" #include "chipdrivers.h"
Index: flashrom-include_cleanup/sharplhf00l04.c =================================================================== --- flashrom-include_cleanup/sharplhf00l04.c (Revision 1017) +++ flashrom-include_cleanup/sharplhf00l04.c (Arbeitskopie) @@ -18,7 +18,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-#include <stdlib.h> #include "flash.h" #include "chipdrivers.h"
Index: flashrom-include_cleanup/nic3com.c =================================================================== --- flashrom-include_cleanup/nic3com.c (Revision 1017) +++ flashrom-include_cleanup/nic3com.c (Arbeitskopie) @@ -21,8 +21,6 @@ #if defined(__i386__) || defined(__x86_64__)
#include <stdlib.h> -#include <string.h> -#include <sys/types.h> #include "flash.h"
#define BIOS_ROM_ADDR 0x04 Index: flashrom-include_cleanup/spi.c =================================================================== --- flashrom-include_cleanup/spi.c (Revision 1017) +++ flashrom-include_cleanup/spi.c (Arbeitskopie) @@ -22,7 +22,6 @@ * Contains the generic SPI framework */
-#include <string.h> #include "flash.h" #include "flashchips.h" #include "chipdrivers.h" Index: flashrom-include_cleanup/satasii.c =================================================================== --- flashrom-include_cleanup/satasii.c (Revision 1017) +++ flashrom-include_cleanup/satasii.c (Arbeitskopie) @@ -21,7 +21,6 @@ /* Datasheets can be found on http://www.siliconimage.com. Great thanks! */
#include <stdlib.h> -#include <string.h> #include "flash.h"
#define PCI_VENDOR_ID_SII 0x1095 Index: flashrom-include_cleanup/wbsio_spi.c =================================================================== --- flashrom-include_cleanup/wbsio_spi.c (Revision 1017) +++ flashrom-include_cleanup/wbsio_spi.c (Arbeitskopie) @@ -20,7 +20,6 @@
#if defined(__i386__) || defined(__x86_64__)
-#include <string.h> #include "flash.h" #include "chipdrivers.h" #include "spi.h" Index: flashrom-include_cleanup/stm50flw0x0x.c =================================================================== --- flashrom-include_cleanup/stm50flw0x0x.c (Revision 1017) +++ flashrom-include_cleanup/stm50flw0x0x.c (Arbeitskopie) @@ -27,8 +27,6 @@ * ST M50FLW080B (not yet tested) */
-#include <string.h> -#include <stdlib.h> #include "flash.h" #include "flashchips.h" #include "chipdrivers.h" Index: flashrom-include_cleanup/sst_fwhub.c =================================================================== --- flashrom-include_cleanup/sst_fwhub.c (Revision 1017) +++ flashrom-include_cleanup/sst_fwhub.c (Arbeitskopie) @@ -22,8 +22,6 @@
/* Adapted from the Intel FW hub stuff for 82802ax parts. */
-#include <stdlib.h> -#include <string.h> #include "flash.h" #include "chipdrivers.h"
Index: flashrom-include_cleanup/chipset_enable.c =================================================================== --- flashrom-include_cleanup/chipset_enable.c (Revision 1017) +++ flashrom-include_cleanup/chipset_enable.c (Arbeitskopie) @@ -27,6 +27,7 @@
#define _LARGEFILE64_SOURCE
+#include <unistd.h> #include <stdlib.h> #include <string.h> #include <sys/types.h> Index: flashrom-include_cleanup/sb600spi.c =================================================================== --- flashrom-include_cleanup/sb600spi.c (Revision 1017) +++ flashrom-include_cleanup/sb600spi.c (Arbeitskopie) @@ -23,7 +23,6 @@
#if defined(__i386__) || defined(__x86_64__)
-#include <string.h> #include "flash.h" #include "chipdrivers.h" #include "spi.h" Index: flashrom-include_cleanup/cli_classic.c =================================================================== --- flashrom-include_cleanup/cli_classic.c (Revision 1017) +++ flashrom-include_cleanup/cli_classic.c (Arbeitskopie) @@ -21,6 +21,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <stdio.h> #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> Index: flashrom-include_cleanup/programmer.c =================================================================== --- flashrom-include_cleanup/programmer.c (Revision 1017) +++ flashrom-include_cleanup/programmer.c (Arbeitskopie) @@ -18,7 +18,6 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-#include <stdlib.h> #include "flash.h"
/* No-op shutdown() for programmers which don't need special handling */ Index: flashrom-include_cleanup/flashrom.c =================================================================== --- flashrom-include_cleanup/flashrom.c (Revision 1017) +++ flashrom-include_cleanup/flashrom.c (Arbeitskopie) @@ -21,6 +21,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <stdio.h> #include <fcntl.h> #include <sys/types.h> #include <sys/stat.h> Index: flashrom-include_cleanup/layout.c =================================================================== --- flashrom-include_cleanup/layout.c (Revision 1017) +++ flashrom-include_cleanup/layout.c (Arbeitskopie) @@ -18,6 +18,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <stdio.h> #include <stdlib.h> #include <string.h> #include <ctype.h> @@ -73,7 +74,7 @@ mb_vendor_offset = *(walk - 2); if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size || mb_part_offset > size || mb_vendor_offset > size) { - printf("Flash image seems to be a legacy BIOS. Disabling checks.\n"); + msg_pinfo("Flash image seems to be a legacy BIOS. Disabling checks.\n"); return 0; }
@@ -81,25 +82,25 @@ mb_vendor = (char *)(bios + size - mb_vendor_offset); if (!isprint((unsigned char)*mb_part) || !isprint((unsigned char)*mb_vendor)) { - printf("Flash image seems to have garbage in the ID location." + msg_pinfo("Flash image seems to have garbage in the ID location." " Disabling checks.\n"); return 0; }
- printf_debug("coreboot last image size " + msg_pdbg("coreboot last image size " "(not ROM size) is %d bytes.\n", *walk);
mainboard_part = strdup(mb_part); mainboard_vendor = strdup(mb_vendor); - printf_debug("Manufacturer: %s\n", mainboard_vendor); - printf_debug("Mainboard ID: %s\n", mainboard_part); + msg_pdbg("Manufacturer: %s\n", mainboard_vendor); + msg_pdbg("Mainboard ID: %s\n", mainboard_part);
/* * If lb_vendor is not set, the coreboot table was * not found. Nor was -m VENDOR:PART specified. */ if (!lb_vendor || !lb_part) { - printf("Note: If the following flash access fails, " + msg_pinfo("Note: If the following flash access fails, " "try -m <vendor>:<mainboard>.\n"); return 0; } @@ -109,13 +110,13 @@ */ if (!strcasecmp(mainboard_vendor, lb_vendor) && !strcasecmp(mainboard_part, lb_part)) { - printf_debug("This firmware image matches this mainboard.\n"); + msg_pdbg("This firmware image matches this mainboard.\n"); } else { if (force_boardmismatch) { - printf("WARNING: This firmware image does not " + msg_pinfo("WARNING: This firmware image does not " "seem to fit to this machine - forcing it.\n"); } else { - printf("ERROR: Your firmware image (%s:%s) does not " + msg_pinfo("ERROR: Your firmware image (%s:%s) does not " "appear to\n be correct for the detected " "mainboard (%s:%s)\n\nOverride with -p internal:" "boardmismatch=force if you are absolutely sure " @@ -141,7 +142,7 @@ romlayout = fopen(name, "r");
if (!romlayout) { - fprintf(stderr, "ERROR: Could not open ROM layout (%s).\n", + msg_gerr("ERROR: Could not open ROM layout (%s).\n", name); return -1; } @@ -159,7 +160,7 @@ tstr1 = strtok(tempstr, ":"); tstr2 = strtok(NULL, ":"); if (!tstr1 || !tstr2) { - fprintf(stderr, "Error parsing layout file.\n"); + msg_gerr("Error parsing layout file.\n"); fclose(romlayout); return 1; } @@ -170,7 +171,7 @@ }
for (i = 0; i < romimages; i++) { - printf_debug("romlayout %08x - %08x named %s\n", + msg_gdbg("romlayout %08x - %08x named %s\n", rom_entries[i].start, rom_entries[i].end, rom_entries[i].name); } @@ -187,16 +188,16 @@ if (!romimages) return -1;
- printf("Looking for "%s"... ", name); + msg_ginfo("Looking for "%s"... ", name);
for (i = 0; i < romimages; i++) { if (!strcmp(rom_entries[i].name, name)) { rom_entries[i].included = 1; - printf("found.\n"); + msg_ginfo("found.\n"); return i; } } - printf("not found.\n"); // Not found. Error. + msg_ginfo("not found.\n"); // Not found. Error.
return -1; } Index: flashrom-include_cleanup/udelay.c =================================================================== --- flashrom-include_cleanup/udelay.c (Revision 1017) +++ flashrom-include_cleanup/udelay.c (Arbeitskopie) @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <unistd.h> #include <sys/time.h> #include <stdlib.h> #include <limits.h> @@ -63,7 +64,7 @@ unsigned long timeusec; int i, tries = 0;
- printf("Calibrating delay loop... "); + msg_pinfo("Calibrating delay loop... ");
recalibrate: count = 1000; @@ -113,7 +114,7 @@ timeusec = measure_delay(10000); msg_pdbg("10000 myus = %ld us, ", timeusec);
- printf("OK.\n"); + msg_pinfo("OK.\n"); }
void internal_delay(int usecs) Index: flashrom-include_cleanup/82802ab.c =================================================================== --- flashrom-include_cleanup/82802ab.c (Revision 1017) +++ flashrom-include_cleanup/82802ab.c (Arbeitskopie) @@ -26,8 +26,6 @@ * - Order number: 290658-004 */
-#include <string.h> -#include <stdlib.h> #include "flash.h" #include "chipdrivers.h"
Index: flashrom-include_cleanup/cbtable.c =================================================================== --- flashrom-include_cleanup/cbtable.c (Revision 1017) +++ flashrom-include_cleanup/cbtable.c (Arbeitskopie) @@ -22,6 +22,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <unistd.h> +#include <stdio.h> #include <stdlib.h> #include <sys/types.h> #include <string.h> Index: flashrom-include_cleanup/print.c =================================================================== --- flashrom-include_cleanup/print.c (Revision 1017) +++ flashrom-include_cleanup/print.c (Arbeitskopie) @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
+#include <stdio.h> #include <string.h> #include <stdlib.h> #include "flash.h" Index: flashrom-include_cleanup/dediprog.c =================================================================== --- flashrom-include_cleanup/dediprog.c (Revision 1017) +++ flashrom-include_cleanup/dediprog.c (Arbeitskopie) @@ -18,9 +18,6 @@ */
#include <string.h> -#include <stdlib.h> -#include <ctype.h> -#include <sys/types.h> #include <usb.h> #include "flash.h" #include "chipdrivers.h" Index: flashrom-include_cleanup/board_enable.c =================================================================== --- flashrom-include_cleanup/board_enable.c (Revision 1017) +++ flashrom-include_cleanup/board_enable.c (Arbeitskopie) @@ -25,7 +25,6 @@ */
#include <string.h> -#include <fcntl.h> #include "flash.h"
#if defined(__i386__) || defined(__x86_64__) @@ -107,8 +106,8 @@ OUTB(0x55, port); /* enter conf mode */ id = sio_read(port, 0x20); if (id != 0x44) { - fprintf(stderr, "\nERROR: %s: FDC37B787: Wrong ID 0x%02X.\n", - name, id); + msg_perr("\nERROR: %s: FDC37B787: Wrong ID 0x%02X.\n", + name, id); OUTB(0xAA, port); /* leave conf mode */ return -1; } @@ -118,8 +117,8 @@ val = sio_read(port, 0xC8); /* GP50 */ if ((val & 0x1B) != 0x10) /* output, no invert, GPIO */ { - fprintf(stderr, "\nERROR: %s: GPIO50 mode 0x%02X unexpected.\n", - name, val); + msg_perr("\nERROR: %s: GPIO50 mode 0x%02X unexpected.\n", + name, val); OUTB(0xAA, port); return -1; }
On Sun, May 30, 2010 at 03:06:31AM +0200, Carl-Daniel Hailfinger wrote:
Compile tests on non-x86 and non-Linux platforms highly appreciated.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Compile-tested on x86 Linux only, though.
Index: flashrom-include_cleanup/physmap.c
--- flashrom-include_cleanup/physmap.c (Revision 1017) +++ flashrom-include_cleanup/physmap.c (Arbeitskopie)
[...]
@@ -311,7 +313,7 @@ { char msrfilename[64]; memset(msrfilename, 0, 64);
Maybe we should also replace this hardcoded 64 with a sizeof()?
- sprintf(msrfilename, "/dev/cpu/%d/msr", cpu);
snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu/%d/msr", cpu);
if (fd_msr != -1) { msg_pinfo("MSR was already initialized\n");
@@ -394,7 +396,7 @@ { char msrfilename[64]; memset(msrfilename, 0, 64);
Ditto.
- sprintf(msrfilename, "/dev/cpu%d", cpu);
snprintf(msrfilename, sizeof(msrfilename), "/dev/cpu%d", cpu);
if (fd_msr != -1) { msg_pinfo("MSR was already initialized\n");
Uwe.
On 30.05.2010 13:55, Uwe Hermann wrote:
On Sun, May 30, 2010 at 03:06:31AM +0200, Carl-Daniel Hailfinger wrote:
Compile tests on non-x86 and non-Linux platforms highly appreciated.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Thanks for the review. I added the changes you suggested and fixed compilation on FreeBSD. Committed in r1021.
Compile-tested on x86 Linux only, though.
Vladimir Serbinenko tested on Yeeloong and FreeBSD
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Uwe Hermann