Skip to contentSkip to conversation

kexec-tools, ppc64: fix build error on ppc64

PATCH In review Michael Neuling 4 participants 11 messages May 5, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kexec-tools, ppc64: fix build error on ppc64

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

---
I've posted a second patch to fix the issue of changing one archs Makefile, effecting all other archs.

 kexec/arch/ppc64/fs2dt.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kexec/arch/ppc64/fs2dt.c +2 -1
Index: kexec-tools/kexec/arch/ppc64/fs2dt.c
===================================================================
@@ -357,7 +357,8 @@ static void putprops(char *fn, struct di
  * Compare function used to sort the device-tree directories
  * This function will be passed to scandir.
  */
-static int comparefunc(const void *dentry1, const void *dentry2)
+static int comparefunc(const struct dirent **dentry1,
+		       const struct dirent **dentry2)
 {
 	char *str1 = (*(struct dirent **)dentry1)->d_name;
 	char *str2 = (*(struct dirent **)dentry2)->d_name;
#1 lore raw

PATCH 1/1 kexec-tools: don't include every archs Makefile

This patches changes the kexec/Makefile to only include the required arch Makefile.

Including all arch Makefiles causes unexpected results as changing one archs Makefile effects all other archs.

ppc64 recently broke surprisingly because 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" added "CFLAGS +=-Wall -Werror" to kexec/arch/mips/Makefile. This shouldn't happen.

---
Horms: Tested by compiling on x86 and ppc64, but probably needs testing on other archs before being committed.

 kexec/Makefile |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)
kexec/Makefile +1 -11
Index: kexec-tools/kexec/Makefile
===================================================================
@@ -48,17 +48,7 @@ KEXEC_SRCS += $($(ARCH)_ARCH_REUSE_INITR
 $(ARCH)_ARCH_INIT		= kexec/arch_init.c
 KEXEC_SRCS += $($(ARCH)_ARCH_INIT)
 
-include $(srcdir)/kexec/arch/alpha/Makefile
-include $(srcdir)/kexec/arch/arm/Makefile
-include $(srcdir)/kexec/arch/i386/Makefile
-include $(srcdir)/kexec/arch/ia64/Makefile
-include $(srcdir)/kexec/arch/mips/Makefile
-include $(srcdir)/kexec/arch/cris/Makefile
-include $(srcdir)/kexec/arch/ppc/Makefile
-include $(srcdir)/kexec/arch/ppc64/Makefile
-include $(srcdir)/kexec/arch/s390/Makefile
-include $(srcdir)/kexec/arch/sh/Makefile
-include $(srcdir)/kexec/arch/x86_64/Makefile
+include $(srcdir)/kexec/arch/$(ARCH)/Makefile
 
 KEXEC_SRCS += $($(ARCH)_KEXEC_SRCS)
 

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw

Re: [PATCH] kexec-tools: don't include every archs Makefile

Quoted 11 lines · Michael Neuling

This patches changes the kexec/Makefile to only include the required arch Makefile.

Including all arch Makefiles causes unexpected results as changing one archs Makefile effects all other archs.

ppc64 recently broke surprisingly because 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" added "CFLAGS +=-Wall -Werror" to kexec/arch/mips/Makefile. This shouldn't happen.

Quoted 4 lines

---
Horms: Tested by compiling on x86 and ppc64, but probably needs testing on other archs before being committed.

Unfortunately this breaks make tarball - only kexec/arch/$ARCH is included in the tarball.

Perhaps we could introduce $(ARCH)_FLAGS, which I think is consistent with the handling of other variables by the kexec build.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#3 lore raw

Re: [PATCH] kexec-tools: don't include every archs Makefile

Quoted 12 lines · In message <[email protected]> you wrote:
Quoted 11 lines · Michael Neuling

This patches changes the kexec/Makefile to only include the required arch Makefile.

Including all arch Makefiles causes unexpected results as changing one archs Makefile effects all other archs.

ppc64 recently broke surprisingly because 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" added "CFLAGS +=-Wall -Werror" to kexec/arch/mips/Makefile. This shouldn't happen.

Quoted 7 lines
Quoted 4 lines

---
Horms: Tested by compiling on x86 and ppc64, but probably needs testing on other archs before being committed.

Unfortunately this breaks make tarball - only kexec/arch/$ARCH is included in the tarball.

I figured it might break something :-)

BTW make tarball works here.

Quoted 2 lines

Perhaps we could introduce $(ARCH)_FLAGS, which I think is consistent with the handling of other variables by the kexec build.

Seems like a bit of a band aid.

It still seems pretty broken that we use arch/mips/Makefile for ppc64.

Looking at 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 and trying to work out how it broke ppc64 was pretty annoying.

Mikey

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#4 lore raw

Re: [PATCH] kexec-tools: don't include every archs Makefile

Quoted 30 lines · Michael Neuling
Quoted 12 lines · In message <[email protected]> you wrote:
Quoted 11 lines · Michael Neuling

This patches changes the kexec/Makefile to only include the required arch Makefile.

Including all arch Makefiles causes unexpected results as changing one archs Makefile effects all other archs.

ppc64 recently broke surprisingly because 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" added "CFLAGS +=-Wall -Werror" to kexec/arch/mips/Makefile. This shouldn't happen.

Quoted 7 lines
Quoted 4 lines

---
Horms: Tested by compiling on x86 and ppc64, but probably needs testing on other archs before being committed.

Unfortunately this breaks make tarball - only kexec/arch/$ARCH is included in the tarball.

I figured it might break something :-)

BTW make tarball works here.

Quoted 2 lines

Perhaps we could introduce $(ARCH)_FLAGS, which I think is consistent with the handling of other variables by the kexec build.

Quoted 6 lines

Seems like a bit of a band aid.

It still seems pretty broken that we use arch/mips/Makefile for ppc64.

Looking at 6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 and trying to work out how it broke ppc64 was pretty annoying.

I agree that its entirely non-obvious and could be improved :-)

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#5 lore raw
In reply to Michael Neuling
Quoted 18 lines · In article <[email protected]> Michael Neuling wrote:

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

---
I've posted a second patch to fix the issue of changing one archs Makefile, effecting all other archs.

 kexec/arch/ppc64/fs2dt.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: kexec-tools/kexec/arch/ppc64/fs2dt.c
===================================================================

kexec/arch/ppc64/fs2dt.c static void putprops(char *fn, struct di · line 364

Quoted 9 lines
kexec/arch/ppc64/fs2dt.c +2 -1
@@ -357,7 +357,8 @@ static void putprops(char *fn, struct di
  * Compare function used to sort the device-tree directories
  * This function will be passed to scandir.
  */
-static int comparefunc(const void *dentry1, const void *dentry2)
+static int comparefunc(const struct dirent **dentry1,
+		       const struct dirent **dentry2)
 {
 	char *str1 = (*(struct dirent **)dentry1)->d_name;
 	char *str2 = (*(struct dirent **)dentry2)->d_name;

It would be nice to also remove the no-longer-needed casts.

-Dale

#6 lore raw
In reply to Michael Neuling
Quoted 4 lines · Michael Neuling

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

Thanks.

While I'm very much in favour of using -Werror for testing and these kind of patches are great, I think that -Werror by default is rather silly. We can't control what will be a warning in the future and sometimes fixing warnings isn't appropriate - for instance there are warnings in fs2dt.c at the moment, that is currently considered an upstream problem.

In short, this patch is great. But I plan to also remove -Werror as the default now that its been brought to my attention.

Quoted 24 lines

---
I've posted a second patch to fix the issue of changing one archs Makefile, effecting all other archs.

 kexec/arch/ppc64/fs2dt.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: kexec-tools/kexec/arch/ppc64/fs2dt.c
===================================================================

kexec/arch/ppc64/fs2dt.c +2 -1
@@ -357,7 +357,8 @@ static void putprops(char *fn, struct di
  * Compare function used to sort the device-tree directories
  * This function will be passed to scandir.
  */
-static int comparefunc(const void *dentry1, const void *dentry2)
+static int comparefunc(const struct dirent **dentry1,
+		       const struct dirent **dentry2)
 {
 	char *str1 = (*(struct dirent **)dentry1)->d_name;
 	char *str2 = (*(struct dirent **)dentry2)->d_name;
Quoted 4 lines

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#7 lore raw
Quoted 15 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Michael Neuling

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

Thanks.

While I'm very much in favour of using -Werror for testing and these kind of patches are great, I think that -Werror by default is rather silly. We can't control what will be a warning in the future and sometimes fixing warnings isn't appropriate - for instance there are warnings in fs2dt.c at the moment, that is currently considered an upstream problem.

Quoted 2 lines

In short, this patch is great. But I plan to also remove -Werror as the default now that its been brought to my attention.

Cool. My second patch should do this for all but MIPS.

Mikey

Quoted 30 lines
Quoted 29 lines

---
I've posted a second patch to fix the issue of changing one archs Makefile, effecting all other archs.

 kexec/arch/ppc64/fs2dt.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: kexec-tools/kexec/arch/ppc64/fs2dt.c
===================================================================

kexec/arch/ppc64/fs2dt.c +2 -1
@@ -357,7 +357,8 @@ static void putprops(char *fn, struct di
  * Compare function used to sort the device-tree directories
  * This function will be passed to scandir.
  */
-static int comparefunc(const void *dentry1, const void *dentry2)
+static int comparefunc(const struct dirent **dentry1,
+		       const struct dirent **dentry2)
 {
 	char *str1 = (*(struct dirent **)dentry1)->d_name;
 	char *str2 = (*(struct dirent **)dentry2)->d_name;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#8 lore raw
Quoted 18 lines · Michael Neuling
Quoted 15 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Michael Neuling

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

Thanks.

While I'm very much in favour of using -Werror for testing and these kind of patches are great, I think that -Werror by default is rather silly. We can't control what will be a warning in the future and sometimes fixing warnings isn't appropriate - for instance there are warnings in fs2dt.c at the moment, that is currently considered an upstream problem.

Quoted 4 lines
Quoted 2 lines

In short, this patch is great. But I plan to also remove -Werror as the default now that its been brought to my attention.

Cool. My second patch should do this for all but MIPS.

Unfortunately the second patch breaks make tarball. For now I have committed the following:

Quoted 1 line

From 76beff0507360f3001e23c9b4c765ef56384d5d5 Mon Sep 17 00:00:00 2001

From: Simon Horman <[email protected]>
Date: Wed, 5 May 2010 13:33:21 +1000
Subject: [PATCH] build: Don't modify CFLAGS in kexec/arch/mips/Makefile

Revert this portion of recent MIPS changes as it actually effects the CFLAGS for all architectures. Moreover I'm somewhat dubious about defaulting to -Werror for any architecture.

---
 kexec/arch/mips/Makefile |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
kexec/arch/mips/Makefile +0 -2
index 831b263..03bdb9a 100644
@@ -14,5 +14,3 @@ dist += kexec/arch/mips/Makefile $(mips_KEXEC_SRCS)			\
 	kexec/arch/mips/kexec-mips.h					\
 	kexec/arch/mips/crashdump-mips.h				\
 	kexec/arch/mips/include/arch/options.h
-
-CFLAGS +=-Wall -Werror
Signature · 1 line
1.7.1
#9 lore raw

2010/5/5 Simon Horman <[email protected]>

Quoted 53 lines · Michael Neuling
Quoted 18 lines
Quoted 15 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Michael Neuling

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

Thanks.

While I'm very much in favour of using -Werror for testing and these kind of patches are great, I think that -Werror by default is rather silly. We can't control what will be a warning in the future and sometimes fixing warnings isn't appropriate - for instance there are warnings in fs2dt.c at the moment, that is currently considered an upstream problem.

Quoted 4 lines
Quoted 2 lines

In short, this patch is great. But I plan to also remove -Werror as the default now that its been brought to my attention.

Cool. My second patch should do this for all but MIPS.

Unfortunately the second patch breaks make tarball. For now I have committed the following:

From 76beff0507360f3001e23c9b4c765ef56384d5d5 Mon Sep 17 00:00:00 2001
From: Simon Horman <[email protected]>
Date: Wed, 5 May 2010 13:33:21 +1000
Subject: [PATCH] build: Don't modify CFLAGS in kexec/arch/mips/Makefile

Revert this portion of recent MIPS changes as it actually effects the CFLAGS for all architectures. Moreover I'm somewhat dubious about defaulting to -Werror for any architecture.

---
 kexec/arch/mips/Makefile |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
kexec/arch/mips/Makefile +0 -3
index 831b263..03bdb9a 100644
@@ -14,5 +14,3 @@ dist += kexec/arch/mips/Makefile $(mips_KEXEC_SRCS)
            \
       kexec/arch/mips/kexec-mips.h                                    \
       kexec/arch/mips/crashdump-mips.h                                \
       kexec/arch/mips/include/arch/options.h
-
-CFLAGS +=-Wall -Werror
--
Quoted 3 lines

1.7.1

Plan was to have -Werror only for mips. Thanks for fixes.

Signature · 2 lines
Best regards,
Maxim Uvarov
#10 lore raw
Quoted 43 lines · Maxim Uvarov

2010/5/5 Simon Horman <[email protected]>

Quoted 41 lines · Michael Neuling
Quoted 18 lines
Quoted 15 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Michael Neuling

6adc05c6e3fdbc8b9f5d915af78ca05d0a09cb17 "some kexec MIPS improvements" broke pp64 as it turned on -Werror for all archs.

This fixes the warning and hence ppc64 building.

Thanks.

While I'm very much in favour of using -Werror for testing and these kind of patches are great, I think that -Werror by default is rather silly. We can't control what will be a warning in the future and sometimes fixing warnings isn't appropriate - for instance there are warnings in fs2dt.c at the moment, that is currently considered an upstream problem.

Quoted 4 lines
Quoted 2 lines

In short, this patch is great. But I plan to also remove -Werror as the default now that its been brought to my attention.

Cool. My second patch should do this for all but MIPS.

Unfortunately the second patch breaks make tarball. For now I have committed the following:

From 76beff0507360f3001e23c9b4c765ef56384d5d5 Mon Sep 17 00:00:00 2001
From: Simon Horman <[email protected]>
Date: Wed, 5 May 2010 13:33:21 +1000
Subject: [PATCH] build: Don't modify CFLAGS in kexec/arch/mips/Makefile

Revert this portion of recent MIPS changes as it actually effects the CFLAGS for all architectures. Moreover I'm somewhat dubious about defaulting to -Werror for any architecture.

---
 kexec/arch/mips/Makefile |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
Quoted 16 lines
Quoted 12 lines
kexec/arch/mips/Makefile +0 -3
index 831b263..03bdb9a 100644
@@ -14,5 +14,3 @@ dist += kexec/arch/mips/Makefile $(mips_KEXEC_SRCS)
            \
       kexec/arch/mips/kexec-mips.h                                    \
       kexec/arch/mips/crashdump-mips.h                                \
       kexec/arch/mips/include/arch/options.h
-
-CFLAGS +=-Wall -Werror
--
Quoted 3 lines

1.7.1

Plan was to have -Werror only for mips. Thanks for fixes.

No problem, I should have noticed this earlier.