This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 2/2 gdb] Fix --enable-libctf, --disable-static, and tests sans CTF-capable toolchain
>>>>> "Nick" == Nick Alcock <nick.alcock@oracle.com> writes:
Thanks for doing this. This looks basically ok, just a few nits below.
The patch should have some intro text. gdb style is to use the same
text for the description in the email and for the commit.
Nick> +if test x${enable_static} = xno; then
Nick> + LIBCTF="-Wl,--rpath,../libctf/.libs ../libctf/.libs/libctf.so"
Indentation of 2 here...
Nick> +if test "${enable_libctf}" = yes; then
Nick> + AC_DEFINE(ENABLE_LIBCTF, 1, [Handle .ctf type-info sections])
... but 4 here. I think they could be the same, whatever is most common
in the file, or most common nearby anyhow.
Nick> +#else
Nick> +
Nick> +void
Nick> +_initialize_ctfread (void)
gdb just uses "()", not "(void)" now.
Nick> diff --git a/gdb/testsuite/configure.ac b/gdb/testsuite/configure.ac
...
Nick> +AC_ARG_ENABLE(libctf,
Nick> +[AS_HELP_STRING([--enable-libctf], [Handle .ctf type-info sections])], [
This seems to be identical to what's in gdb.
I don't really mind, but if you're duplicating this across several
subdirectories, it may be better to stick a new macro into a new .m4 in
config/, then use that everywhere.
Nick> +if {[skip_ctf_tests]} {
Nick> + return
Nick> +}
I think these spots should call "unsupported" to explain what happened.
Nick> +proc skip_ctf_tests {} {
I suspect you probably want to use gdb_caching_proc here. This can be
friendlier when running a lot of parallel tests, because it caches the
result, and this proc is doing a compilation.
Tom