From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: GDB Patches <gdb-patches@sourceware.org>,
Pedro Alves <palves@redhat.com>, John Baldwin <jhb@freebsd.org>
Subject: Re: [PATCH] Introduce "gdb/configure.nat" (and delete "gdb/config/*/*.mh" files)
Date: Tue, 02 May 2017 22:17:00 -0000 [thread overview]
Message-ID: <87wp9ygaze.fsf@redhat.com> (raw)
In-Reply-To: <f99c9eb03801ea296fe14b42b9515e14@polymtl.ca> (Simon Marchi's message of "Tue, 02 May 2017 16:16:27 -0400")
On Tuesday, May 02 2017, Simon Marchi wrote:
> On 2017-05-02 15:28, Sergio Durigan Junior wrote:
>>>> +# Variables defined here:
>>>> +#
>>>> +# NAT_FILE
>>>> +# NATDEPFILES
>>>> +# NAT_CDEPS
>>>> +# LOADLIBES
>>>> +# MH_CFLAGS
>>>> +# XM_CLIBS
>>>> +# NAT_GENERATED_FILES
>>>> +# HAVE_NATIVE_GCORE_HOST
>>>> +# NAT_EXTRA_FRAGS_FILE
>>>
>>> Could you document (if you know it) what each variable does?
>>
>> Sure. Here's what I know what them. What do you think?
>>
>> # Variables defined here:
>> #
>> # NAT_FILE - The header file with definitions for this host
>
> Looks good.
>
>> #
>> # NATDEPFILES - The depfiles to be compiled for this host
>
> "depfiles" is not so clear I think. Perhaps
>
> NATDEPFILES - Source files required for native debugging on this host.
Adopted.
>> #
>> # NAT_CDEPS - Dynamic symbols to be exported for libthread_db
>
> Looks good.
>
>> #
>> # LOADLIBES - Libraries that will be linked against GDB for this host
>
> Don't we usually say the reverse, the program is linked against the
> libraries?
>
> LOADLIBES - Libraries against which GDB will be linked for this host.
You're right. Fixed.
>> #
>> # MH_CFLAGS - Additional CFLAGS for this host
>
> Looks good.
>
>> #
>> # XM_CLIBS - Host-dependent libs to be linked against GDB.
>
> Same comment about "linked against".
Fixed.
>> # NAT_GENERATED_FILES - Generated files by this host
>
> by -> for?
>
> NAT_GENERATED_FILES - Generated files for this host.
Right, fixed.
>> # HAVE_NATIVE_GCORE_HOST -
>
> I just noticed that this variable is not defined in Makefile.in.
> configure.ac AC_SUBST's _TARGET twice, but not _HOST, so I think you
> confused them.
Yeah, I did confuse. Fixed on gdb/Makefile.in.
> But to be honest, I am also completely confused by what those two
> variables do.
So, doing a bit of archaeology I found that these flags were added by
b292c7831823605b02a564b0a9274f95141c92ee. According to the discussions
relevant to this commit
(<https://sourceware.org/ml/gdb-patches/2013-04/msg00321.html>), I see
that these flags serve to signalize whether the gcore script will be
installed or not.
>> # NAT_EXTRA_FRAGS_FILE - Extra Makefile fragmentsFile containing
>> extra fragments of Makefile
>> # that will be used by this host.
>
> That could be shortened to
>
> NAT_EXTRA_FRAGS_FILE - Extra Makefile fragments that will be used
> for this host.
Hm, not sure. My intention was to make it clear that this variable
needs to contain a filename. Your proposed description could be
interpreted differently, I'm afraid.
> Make sure they all end (or don't end) with a period, for consistency.
Sure; this was just a quick draft, I fixed the inconsistencies now :-).
> Perhaps someone could shed some light on the obscure variables names?
> What do the XM_ and MH_ prefixes stand for? What's the difference
> between XM_CLIBS and LOADLIBES (and why is it called "LIBES"?) ?
Good question, I was also wondering that myself :-).
>>> IMO, the interest of having all of this in a single file is to be able
>>> to factor out common things. A lot of NATDEPFILES are repeated Would
>>> it be possible to have a switch on ${gdb_host} at the top level, and
>>> specify all the files specific to OSes but machine-agnostic? For
>>> example, fork-child.o and inf-ptrace.o probably appear in all the
>>> linux ports.
>>
>> Fair point. I will address what you and Jon mentioned, and resubmit
>> the
>> patch with the modifications.
>
> Ok. But as John mentioned, it's probably better if this patch is a
> simple translation of what's already there, and any "optimization"
> coming in a subsequent patch. It will be easier to track the changes.
Sure. I'll send a series of 2 patches, the first one containing the
original patch (+ the proposed modifications by you and Pedro), and the
second one containing the rearrangement to make things clearer.
Cheers,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
next prev parent reply other threads:[~2017-05-02 22:17 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-25 20:23 Sergio Durigan Junior
2017-04-25 21:08 ` John Baldwin
2017-05-01 18:45 ` Sergio Durigan Junior
2017-05-02 2:44 ` Simon Marchi
2017-05-02 14:57 ` John Baldwin
2017-05-02 17:01 ` Simon Marchi
2017-05-02 19:28 ` Sergio Durigan Junior
2017-05-02 20:16 ` Simon Marchi
2017-05-02 21:30 ` Pedro Alves
2017-05-02 22:17 ` Sergio Durigan Junior [this message]
2017-05-03 3:49 ` [PATCH v2 0/2] Introduce gdb/configure.nat Sergio Durigan Junior
2017-05-03 3:49 ` [PATCH v2 1/2] Introduce "gdb/configure.nat" (and delete "gdb/config/*/*.mh" files) Sergio Durigan Junior
2017-05-04 16:16 ` Pedro Alves
2017-05-05 3:58 ` Sergio Durigan Junior
2017-05-05 9:41 ` Pedro Alves
2017-05-06 14:04 ` Sergio Durigan Junior
2017-05-17 14:03 ` Pedro Alves
2017-05-03 3:49 ` [PATCH v2 2/2] Rearrange gdb/configure.nat to make it simpler and less redundant Sergio Durigan Junior
2017-05-03 16:45 ` John Baldwin
2017-05-03 17:28 ` Sergio Durigan Junior
2017-05-04 16:34 ` Pedro Alves
2017-05-05 4:23 ` Sergio Durigan Junior
2017-05-05 4:31 ` [PATCH v3 0/2] Introduce gdb/configure.nat Sergio Durigan Junior
2017-05-05 4:31 ` [PATCH v3 2/2] Rearrange gdb/configure.nat to make it simpler and less redundant Sergio Durigan Junior
2017-05-06 14:13 ` Sergio Durigan Junior
2017-05-05 4:32 ` [PATCH v3 1/2] Introduce "gdb/configure.nat" (and delete "gdb/config/*/*.mh" files) Sergio Durigan Junior
2017-05-05 16:35 ` Pedro Alves
2017-05-06 14:13 ` Sergio Durigan Junior
2017-05-17 13:22 ` Pedro Alves
2017-05-23 14:40 ` Sergio Durigan Junior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wp9ygaze.fsf@redhat.com \
--to=sergiodj@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=jhb@freebsd.org \
--cc=palves@redhat.com \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox