Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Sergio Durigan Junior <sergiodj@redhat.com>
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 20:16:00 -0000	[thread overview]
Message-ID: <f99c9eb03801ea296fe14b42b9515e14@polymtl.ca> (raw)
In-Reply-To: <8737cngise.fsf@redhat.com>

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.

> #
> # 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.

> #
> # MH_CFLAGS - Additional CFLAGS for this host

Looks good.

> #
> # XM_CLIBS - Host-dependent libs to be linked against GDB.

Same comment about "linked against".

> # NAT_GENERATED_FILES - Generated files by this host

by -> for?

   NAT_GENERATED_FILES - Generated files for this host.

> # 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.

But to be honest, I am also completely confused by what those two 
variables do.

> # 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.

Make sure they all end (or don't end) with a period, for consistency.

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"?) ?

>> 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.

Thanks,

Simon


  reply	other threads:[~2017-05-02 20:16 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 [this message]
2017-05-02 21:30       ` Pedro Alves
2017-05-02 22:17       ` Sergio Durigan Junior
2017-05-03  3:49 ` [PATCH v2 0/2] Introduce gdb/configure.nat Sergio Durigan Junior
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-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-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=f99c9eb03801ea296fe14b42b9515e14@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=jhb@freebsd.org \
    --cc=palves@redhat.com \
    --cc=sergiodj@redhat.com \
    /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