On Tue, 12 Jan 2016, Ian Lepore wrote:
On Wed, 2016-01-13 at 01:17 +0000, Steven Hartland wrote:
On 13/01/2016 00:54, Ian Lepore wrote:
On Wed, 2016-01-13 at 00:43 +0000, Steven Hartland wrote:
...
Passes a full tinderbox so I assume your forcing gcc for some
reason?
For several reasons. The fact that gcc isn't the default compiler
doesn't mean that it's okay for code to not compile with gcc; it's
still a supported compiler for arm.
I usually force gcc.
Not disagreeing with that, was just curious that's all ;-)
The warnings you list seem to be detail, typical gcc, specifically:
sys/boot/efi/fdt/../include/efiapi.h:535: warning: function
declaration isn't a prototype
I'm guessing its being picky and wants EFI_RESERVED_SERVICE to have
void in there due to no params.
It is not broken, so it is warning about an unprototyped function as
requested by -Wstrict-prototypes.
Does the following help:
Index: sys/boot/efi/fdt/Makefile
===================================================================
--- sys/boot/efi/fdt/Makefile (revision 293796)
+++ sys/boot/efi/fdt/Makefile (working copy)
@@ -7,6 +7,8 @@
LIB= efi_fdt
INTERNALLIB=
WARNS?= 6
+CWARNFLAGS.gcc+= -Wno-strict-prototypes
+CWARNFLAGS.gcc+= -Wno-redundant-decls
SRCS= efi_fdt.c
This just breaks the warning.
@@ -34,4 +36,6 @@ CLEANFILES+= machine
.include <bsd.lib.mk>
+CFLAGS+= ${CWARNFLAGS.${COMPILER_TYPE}}
+
beforedepend ${OBJS}: machine
Could you detail detail how you're switching to gcc so I an run a
full pass on that too?
Sometimes I use CC=gccNN, where gccNN is somwhere in $PATH. Sometimes
it is a script to select an arch-dependent gcc. This is unlikely to
work for makeworld but it works for kernels.
Yep, but then I had to do this because ef->off is 64 bits even on 32
bit arches, so I got a pointer/int size mismatch warning...
gcc detects this error too.
Index: common/load_elf.c
===================================================================
--- common/load_elf.c (revision 293796)
+++ common/load_elf.c (working copy)
@@ -886,7 +886,7 @@ __elfN(parse_modmetadata)(struct preloaded_file *f
error = __elfN(reloc_ptr)(fp, ef, v, &md, sizeof(md));
if (error == EOPNOTSUPP) {
md.md_cval += ef->off;
- md.md_data = (void *)((uintptr_t)md.md_data + ef->off);
+ md.md_data = (void *)(uintptr_t)((uintptr_t)md.md_data +
ef->off);
} else if (error != 0)
return (error);
#endif
That is just some special kind of ugly. Fixing warnings is supposed to
lead to better code, but all this casting isn't better, it's just an
unreadable mess. Man I miss the days when C was just a really powerful
macro assembler. :)
This is made extra-ugly by misformatting it. Fixing warnings
unfortunately usually leads to worse code, using extra code to break
the warning.
Here the detected bug is the bogus type for ef->off. Values >=
UINT32_MAX in it cannot work in expressions like this. This was only
detected accidentally.
md_data is a very confusing variable name. It is used nearby in 4 structs
band has a different type in each. Sometimes it is uint32_t or
uint64_t; sometimes it is void * and sometimes it is char *. Here it
is void *.
Some of the structs are:
- mod_medadata (md is this); type void *
- mod_metadata64; type uint64_t
- mod_metadata32; type uint32_t
- file_metadata; type char []
The prefix is supposed to be unique in context. One is better than none.
'off' in ef has none.
The void * type is inconvenient to work with. The nearby md_cval has
the same bugs, except it isn't in file_metadata and its type is
const char * so you can add an integer to it without casting. The
previous round of fixes was to fix a warning about using the gnu
extension of adding an integer to a void * without a cast.
+ md.md_data = (void *)(uintptr_t)((uintptr_t)md.md_data +
ef->off);
I don't see any better quick fix than changing 'off' to vm_offset_t
or maybe signed vm_offset_t. It is in a local struct so this seems
to be possible. Changing the void * instance of md_data to an integer
is harder.
md_cval should also be an integer.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
