[PATCH 0/2] gcc 9 build fixes

[Mark Cave-Ayland]

During the past couple of weeks I've updated my build chain to use gcc 9.1
which generates some additional warnings that cause the build to fail
when building with -Werror.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk>

Mark Cave-Ayland (2):
  all: gcc 9 build fixes
  ppc: reduce size of Forth dictionary to 384K

 arch/ppc/qemu/kernel.c | 4 ++--
 arch/sparc32/context.c | 2 +-
 drivers/cuda.c         | 4 ++--
 drivers/ide.c          | 2 +-
 drivers/pmu.c          | 4 ++--
 fs/hfs/hfs_fs.c        | 2 +-
 packages/nvram.c       | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH 1/2] all: gcc 9 build fixes

[Mark Cave-Ayland]

Fix up warnings generated by building with -Werror under gcc 9.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk>
---
 arch/sparc32/context.c | 2 +-
 drivers/cuda.c         | 4 ++--
 drivers/ide.c          | 2 +-
 drivers/pmu.c          | 4 ++--
 fs/hfs/hfs_fs.c        | 2 +-
 packages/nvram.c       | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/sparc32/context.c b/arch/sparc32/context.c
index 65c421a..b4b0ae3 100644
--- a/arch/sparc32/context.c
+++ b/arch/sparc32/context.c
@@ -115,7 +115,7 @@ struct context *switch_to(struct context *ctx)
     __context = ctx;
     asm __volatile__ ("\n\tcall __switch_context"
                       "\n\tnop" ::: "g1", "g2",
"g3", "g4", "g5", "g6", "g7",
-                      "o0", "o1", "o2", "o3",
"o4", "o5", "sp", "o7",
+                      "o0", "o1", "o2", "o3",
"o4", "o5", "o7",
                       "l0", "l1", "l2", "l3",
"l4", "l5", "l6", "l7",
                       "i0", "i1", "i2", "i3",
"i4", "i5", "i7",
                       "f0", "f1", "f2", "f3",
"f4", "f5", "f6", "f7", "f8",
"f9",
diff --git a/drivers/cuda.c b/drivers/cuda.c
index ff5d22d..d079d1e 100644
--- a/drivers/cuda.c
+++ b/drivers/cuda.c
@@ -369,7 +369,7 @@ static void
 rtc_init(char *path)
 {
 	phandle_t ph, aliases;
-	char buf[64];
+	char buf[128];
 
         snprintf(buf, sizeof(buf), "%s/rtc", path);
 	REGISTER_NAMED_NODE(rtc, buf);
@@ -387,7 +387,7 @@ static void
 powermgt_init(char *path)
 {
 	phandle_t ph;
-	char buf[64];
+	char buf[128];
 
         snprintf(buf, sizeof(buf), "%s/power-mgt", path);
 	REGISTER_NAMED_NODE(rtc, buf);
diff --git a/drivers/ide.c b/drivers/ide.c
index edbd287..519ad46 100644
--- a/drivers/ide.c
+++ b/drivers/ide.c
@@ -987,7 +987,7 @@ ob_ide_identify_drive(struct ide_drive *drive)
 		drive->sect = id.sectors;
 	}
 
-	strncpy(drive->model, (char*)id.model, sizeof(id.model));
+	strncpy(drive->model, (char*)id.model, sizeof(drive->model));
 	drive->model[40] = '\0';
 	return 0;
 }
diff --git a/drivers/pmu.c b/drivers/pmu.c
index 6fc5363..9198846 100644
--- a/drivers/pmu.c
+++ b/drivers/pmu.c
@@ -523,7 +523,7 @@ NODE_METHODS(rtc) = {
 static void rtc_init(char *path)
 {
     phandle_t ph, aliases;
-    char buf[64];
+    char buf[128];
 
     snprintf(buf, sizeof(buf), "%s/rtc", path);
     REGISTER_NAMED_NODE(rtc, buf);
@@ -540,7 +540,7 @@ static void rtc_init(char *path)
 static void powermgt_init(char *path)
 {
     phandle_t ph;
-    char buf[64];
+    char buf[128];
 
     /* This is a bunch of magic "Feature" bits for which we only have
      * partial definitions from Darwin. These are taken from a
diff --git a/fs/hfs/hfs_fs.c b/fs/hfs/hfs_fs.c
index 2e63dbf..ca8a433 100644
--- a/fs/hfs/hfs_fs.c
+++ b/fs/hfs/hfs_fs.c
@@ -86,7 +86,7 @@ _search( hfsvol *vol, const char *path, const char *sname, hfsfile
**ret_fd )
 
 	strncpy( buf, path, sizeof(buf) );
 	if( buf[strlen(buf)-1] != ':' )
-		strncat( buf, ":", sizeof(buf) );
+		strncat( buf, ":", sizeof(buf) - 1 );
 	buf[sizeof(buf)-1] = 0;
 	p = buf + strlen( buf );
 
diff --git a/packages/nvram.c b/packages/nvram.c
index 3182edf..e895fed 100644
--- a/packages/nvram.c
+++ b/packages/nvram.c
@@ -105,7 +105,7 @@ create_free_part( char *ptr, int size )
 	nvpart_t *nvp = (nvpart_t*)ptr;
 	memset( nvp, 0, size );
 
-	strncpy( nvp->name, "777777777777", sizeof(nvp->name) );
+	strncpy( nvp->name, "77777777777", sizeof(nvp->name) );
 	nvp->signature = NV_SIG_FREE;
 	nvp->len_hi = (size /16) >> 8;
 	nvp->len_lo = size /16;
-- 
2.20.1

[PATCH 2/2] ppc: reduce size of Forth dictionary to 384K

[Mark Cave-Ayland]

gcc 9 emits a fatal error when building OpenBIOS for ppc because the size of the
generated image is greater than the available ROM area when building with -O0.

Reduce the size of the Forth dictionary from 512K to 384K to allow sufficient
space for generating OpenBIOS images suitable for debugging.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk>
---
 arch/ppc/qemu/kernel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ppc/qemu/kernel.c b/arch/ppc/qemu/kernel.c
index b26fba5..893ab68 100644
--- a/arch/ppc/qemu/kernel.c
+++ b/arch/ppc/qemu/kernel.c
@@ -26,8 +26,8 @@
 #include "kernel.h"
 
 #define MEMORY_SIZE	(256*1024)	/* 256K ram for hosted system */
-/* 512K for the dictionary  */
-#define DICTIONARY_SIZE (512 * 1024 / sizeof(ucell))
+/* 384K for the dictionary  */
+#define DICTIONARY_SIZE (384 * 1024 / sizeof(ucell))
 #ifdef __powerpc64__
 #define DICTIONARY_BASE 0xfff08000 /* this must match the value in ldscript! */
 #define DICTIONARY_SECTION __attribute__((section(".data.dict")))
-- 
2.20.1

[Mark Cave-Ayland]

On 16/08/2019 08:27, Mark Cave-Ayland wrote:

  During the past couple of weeks I've updated my
build chain to use gcc 9.1
 which generates some additional warnings that cause the build to fail
 when building with -Werror.
 
 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk>
 
 Mark Cave-Ayland (2):
   all: gcc 9 build fixes
   ppc: reduce size of Forth dictionary to 384K
 
  arch/ppc/qemu/kernel.c | 4 ++--
  arch/sparc32/context.c | 2 +-
  drivers/cuda.c         | 4 ++--
  drivers/ide.c          | 2 +-
  drivers/pmu.c          | 4 ++--
  fs/hfs/hfs_fs.c        | 2 +-
  packages/nvram.c       | 2 +-
  7 files changed, 10 insertions(+), 10 deletions(-)
I've pushed this to master since it conflicts with the latest version of the
active-device patchset which I'm about to resend.


ATB,

Mark.

[Programmingkid]

  On Aug 18, 2019, at 3:41 PM, Mark Cave-Ayland
<mark.cave-ayland(a)ilande.co.uk> wrote:
 
 On 16/08/2019 08:27, Mark Cave-Ayland wrote:
 
> During the past couple of weeks I've updated my build chain to use gcc 9.1
> which generates some additional warnings that cause the build to fail
> when building with -Werror.
I'm thinking removing -Werror might be a better option.

[Mark Cave-Ayland]

On 18/08/2019 22:09, Programmingkid wrote:

  On Aug 18,
2019, at 3:41 PM, Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> wrote:

 On 16/08/2019 08:27, Mark Cave-Ayland wrote:

> During the past couple of weeks I've updated my build chain to use gcc 9.1
> which generates some additional warnings that cause the build to fail
> when building with -Werror.
 
 I'm thinking removing -Werror might be a better option.
Why? All of these things flagged up by the compiler are genuine errors.


ATB,

Mark.

[Programmingkid]

  On Aug 18, 2019, at 5:30 PM, Mark Cave-Ayland
<mark.cave-ayland(a)ilande.co.uk> wrote:
 
 On 18/08/2019 22:09, Programmingkid wrote:
 
  On Aug
18, 2019, at 3:41 PM, Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> wrote:
 
 On 16/08/2019 08:27, Mark Cave-Ayland wrote:
 
> During the past couple of weeks I've updated my build chain to use gcc 9.1
> which generates some additional warnings that cause the build to fail
> when building with -Werror.
 
 I'm thinking removing -Werror might be a better option.
 
 Why? All of these things flagged up by the compiler are genuine errors.
 
 
 ATB,
 
 Mark.
The -Werror option treats warnings as errors. I would not consider a warning to be a
genuine error. I always remove -Werror in order for OpenBIOS to compile on my system. So
far this works well.

[BALATON Zoltan]

On Sun, 18 Aug 2019, Programmingkid wrote:
  On Aug 18,
2019, at 5:30 PM, Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> wrote:
 On 18/08/2019 22:09, Programmingkid wrote:
  I'm thinking removing -Werror might be a
better option.
 Why? All of these things flagged up by the compiler are genuine errors.


 ATB,

 Mark.
 The -Werror option treats warnings as errors. I would not consider a 
 warning to be a genuine error. I always remove -Werror in order for 
 OpenBIOS to compile on my system. So far this works well.
These warnings often mean that there may be a problem or at least the code 
is not completely obvious just the compiler is not sure if it's really an 
error or could not exactly identify it and it might work but needs review. 
That's what warning means so definitely not to be ignored. It's a good 
idea to write code that compiles without warnings and is correct without 
any doubt which -Werror ensures. (Especially for portable code. Warnings 
are only acceptable if you need to do something very tricky and you know 
it will work but it's rarely the case that's needed.) If it compiles and 
works with warnings you're lucky but that does not mean the code cannot be 
improved. If you need removing -Werror to compile with your compiler maybe 
a better way is to report those problems so they can be fixed (maybe they 
are actual bugs that gcc did not notice so far) rather than removing 
-Werror and ignoring potential problems.

Regards,
BALATON Zoltan

[Segher Boessenkool]

On Mon, Aug 19, 2019 at 12:13:42PM +0200, BALATON Zoltan wrote:
  On Sun, 18 Aug 2019, Programmingkid wrote:
 The -Werror option treats warnings as errors. I
would not consider a 
warning to be a genuine error. I always remove -Werror in order for 
OpenBIOS to compile on my system. So far this works well.
 
 These warnings often mean that there may be a problem or at least the code 
 is not completely obvious just the compiler is not sure if it's really an 
 error or could not exactly identify it and it might work but needs review. 
 That's what warning means so definitely not to be ignored. It's a good 
 idea to write code that compiles without warnings and is correct without 
 any doubt which -Werror ensures.
No, it usually ensures your code does not compile, instead.

  (Especially for portable code. Warnings 
 are only acceptable if you need to do something very tricky and you know 
 it will work but it's rarely the case that's needed.) If it compiles and 
 works with warnings you're lucky but that does not mean the code cannot be 
 improved. If you need removing -Werror to compile with your compiler maybe 
 a better way is to report those problems so they can be fixed (maybe they 
 are actual bugs that gcc did not notice so far) rather than removing 
 -Werror and ignoring potential problems.
And shipping with -Werror enabled is a disservice to your users.

Users have a different environment than you tested with -- compiler
version, etc.


Segher