Change in flashrom[master]: Fix compilation if CONFIG_INTERNAL=no

[Jonathan Liu (Code Review)]

Jonathan Liu has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
---
M libflashrom.c
M physmap.c
2 files changed, 12 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/35808/1

diff --git a/libflashrom.c b/libflashrom.c
index dbc5129..f3e69ff 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -171,18 +171,21 @@
 struct flashrom_board_info *flashrom_supported_boards(void)
 {
    int boards_known_size = 0;
+#if CONFIG_INTERNAL == 1
    int i = 0;
    const struct board_info *binfo = boards_known;

while ((binfo++)->vendor)
    	++boards_known_size;
    binfo = boards_known;
+#endif
    /* add place for {0} */
    ++boards_known_size;

struct flashrom_board_info *supported_boards =
-		malloc(boards_known_size * sizeof(*binfo));
+		malloc(boards_known_size * sizeof(*supported_boards));

+#if CONFIG_INTERNAL == 1
    if (supported_boards != NULL) {
    	for (; i < boards_known_size; ++i) {
    		supported_boards[i].vendor = binfo[i].vendor;
@@ -192,6 +195,7 @@
    } else {
    	msg_gerr("Memory allocation error!\n");
    }
+#endif

return supported_boards;
 }
@@ -203,18 +207,21 @@
 struct flashrom_chipset_info *flashrom_supported_chipsets(void)
 {
    int chipset_enables_size = 0;
+#if CONFIG_INTERNAL == 1
    int i = 0;
    const struct penable *chipset = chipset_enables;

while ((chipset++)->vendor_name)
    	++chipset_enables_size;
    chipset = chipset_enables;
+#endif
    /* add place for {0}*/
    ++chipset_enables_size;

struct flashrom_chipset_info *supported_chipsets =
    	malloc(chipset_enables_size * sizeof(*supported_chipsets));

+#if CONFIG_INTERNAL == 1
    if (supported_chipsets != NULL) {
    	for (; i < chipset_enables_size; ++i) {
    		supported_chipsets[i].vendor = chipset[i].vendor_name;
@@ -226,6 +233,7 @@
    } else {
    	msg_gerr("Memory allocation error!\n");
    }
+#endif

return supported_chipsets;
 }
diff --git a/physmap.c b/physmap.c
index ad38ad3..c6a2076 100644
--- a/physmap.c
+++ b/physmap.c
@@ -430,6 +430,7 @@
    return 0;
 }

+#if CONFIG_INTERNAL == 1
 int setup_cpu_msr(int cpu)
 {
    char msrfilename[64];
@@ -464,6 +465,8 @@
    /* Clear MSR file descriptor. */
    fd_msr = -1;
 }
+#endif
+
 #elif defined(__OpenBSD__) && defined (__i386__) /* This does only work for certain AMD Geode LX systems see amdmsr(4). */
 #include <sys/ioctl.h>
 #include <machine/amdmsr.h>

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Patch Set 1:

(3 comments)

Hi, thanks for this patch.

I left a few comments about a few things I'm not sure of for Nico to check.

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c 
File libflashrom.c:

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@171 
PS1, Line 171: struct flashrom_board_info *flashrom_supported_boards(void)
I think this function as a whole is specific to the internal programmer

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@207 
PS1, Line 207: struct flashrom_chipset_info *flashrom_supported_chipsets(void)
Same for this one

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c 
File physmap.c:

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c@366 
PS1, Line 366: /* MSR abstraction implementations for Linux, OpenBSD, FreeBSD/Dragonfly, OSX, libpayload
             :  * and a non-working default implementation on the bottom. See also hwaccess.h for some (re)declarations. */
I doubt that anything from here onwards is needed for anything but the internal programmer.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c 
File libflashrom.c:

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@199 
PS1, Line 199: 
Looks like the memory behind `supported_boards` is left uninitialized on
the negative path?

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c 
File physmap.c:

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c@433 
PS1, Line 433: #if CONFIG_INTERNAL == 1
Why does it need a guard? I don't see anything non-standard

[Jonathan Liu (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/35808

to look at the new patch set (#2).

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
---
M libflashrom.c
M physmap.c
2 files changed, 12 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/35808/2

[Jonathan Liu (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/35808

to look at the new patch set (#3).

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
---
M libflashrom.c
M physmap.c
2 files changed, 12 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/35808/3

[Jonathan Liu (Code Review)]

Jonathan Liu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Patch Set 3:

(5 comments)

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c 
File libflashrom.c:

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@171 
PS1, Line 171: struct flashrom_board_info *flashrom_supported_boards(void)
> I think this function as a whole is specific to the internal programmer

Ack

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@199 
PS1, Line 199:
> Looks like the memory behind `supported_boards` is left uninitialized on […]

Ack

https://review.coreboot.org/c/flashrom/+/35808/1/libflashrom.c@207 
PS1, Line 207: struct flashrom_chipset_info *flashrom_supported_chipsets(void)
> Same for this one

Ack

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c 
File physmap.c:

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c@366 
PS1, Line 366: /* MSR abstraction implementations for Linux, OpenBSD, FreeBSD/Dragonfly, OSX, libpayload
             :  * and a non-working default implementation on the bottom. See also hwaccess.h for some (re)declarations. */
> I doubt that anything from here onwards is needed for anything but the internal programmer.

Ack

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c@433 
PS1, Line 433: #if CONFIG_INTERNAL == 1
> Why does it need a guard? I don't see anything non-standard

The prototype for setup_cpu_msr and cleanup_cpu_msr in programmer.h is conditional on CONFIG_INTERNAL == 1

[Jonathan Liu (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/35808

to look at the new patch set (#4).

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
---
M libflashrom.c
M physmap.c
2 files changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/35808/4

[Jonathan Liu (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/35808

to look at the new patch set (#5).

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
---
M libflashrom.c
M physmap.c
2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/35808/5

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Patch Set 5: Code-Review+2

(1 comment)

Will run manibuilder and hopefully merge later today...

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c 
File physmap.c:

https://review.coreboot.org/c/flashrom/+/35808/1/physmap.c@433 
PS1, Line 433: #if CONFIG_INTERNAL == 1
> The prototype for setup_cpu_msr and cleanup_cpu_msr in programmer. […]

I would have preferred to remove the spurious guards in the header
file instead of adding even more guards to the existing mess ;)
I'll try that in a follow-up when testing later.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Patch Set 5: Verified+1

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/35808 )

Change subject: Fix compilation if CONFIG_INTERNAL=no
......................................................................

Fix compilation if CONFIG_INTERNAL=no

Change-Id: Id9e07332003832465a0eccf1d89e73d15abb35c0
Signed-off-by: Jonathan Liu 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/35808
Tested-by: build bot (Jenkins) 
Tested-by: Nico Huber 
Reviewed-by: Nico Huber 
---
M libflashrom.c
M physmap.c
2 files changed, 10 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Verified; Looks good to me, approved

diff --git a/libflashrom.c b/libflashrom.c
index 1d8a9ae..0dec22e 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -170,6 +170,7 @@
  */
 struct flashrom_board_info *flashrom_supported_boards(void)
 {
+#if CONFIG_INTERNAL == 1
    int boards_known_size = 0;
    int i = 0;
    const struct board_info *binfo = boards_known;
@@ -194,6 +195,9 @@
    }

return supported_boards;
+#else
+	return NULL;
+#endif
 }

/**
@@ -202,6 +206,7 @@
  */
 struct flashrom_chipset_info *flashrom_supported_chipsets(void)
 {
+#if CONFIG_INTERNAL == 1
    int chipset_enables_size = 0;
    int i = 0;
    const struct penable *chipset = chipset_enables;
@@ -228,6 +233,9 @@
    }

return supported_chipsets;
+#else
+	return NULL;
+#endif
 }

/**
diff --git a/physmap.c b/physmap.c
index ad38ad3..72d5899 100644
--- a/physmap.c
+++ b/physmap.c
@@ -363,6 +363,7 @@
    return physmap_common(descr, phys_addr, len, PHYSM_RO, PHYSM_NOCLEANUP, PHYSM_EXACT);
 }

+#if CONFIG_INTERNAL == 1
 /* MSR abstraction implementations for Linux, OpenBSD, FreeBSD/Dragonfly, OSX, libpayload
  * and a non-working default implementation on the bottom. See also hwaccess.h for some (re)declarations. */
 #if defined(__i386__) || defined(__x86_64__)
@@ -687,3 +688,4 @@
 #else // x86
 /* Does MSR exist on non-x86 architectures? */
 #endif // arch switches for MSR code
+#endif // CONFIG_INTERNAL == 1