public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Hannes Domani <ssbssa@yahoo.de>,
	Gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Implement debugging of WOW64 processes
Date: Tue, 03 Mar 2020 21:31:00 -0000	[thread overview]
Message-ID: <5c9e53bd-cbd1-cd59-e701-43a1e76aee3b@simark.ca> (raw)
In-Reply-To: <140963475.6133151.1583269956021@mail.yahoo.com>

On 2020-03-03 4:12 p.m., Hannes Domani via gdb-patches wrote:
> I'm not very fond of the code duplication either.
> The only way I can think of, is to refactor it into template functions that
> can accept both CONTEXT or WOW64_CONTEXT.
> Is this what you had in mind also, or do you have a better idea?

I didn't really have something specific in mind.

However, I'm thinking that sprinkling the code with if/else such as:

  /* Do something */
#ifdef __x86_64__
  if (wow64_process)
    {
      ...
    }
#endif
  else
    {
      ...
    }

... does not help with readability and maintenance.  To address that, we could have an interface
with two concrete implementations (one for "standard" processes and one for WOW64 processes).  We
would choose and instantiate the right implementation at runtime (at the place where you currently
set the wow64_process variable), so the code above would become:

  /* Do something */
  impl->do_something ();

where the actual code is in each of the implementations' do_something methods.

That itself wouldn't reduce the code duplication.  But it would keep the specificities of dealing
with a standard process vs a WOW64 process at a single place.

To reduce code duplication, if there is any chance of sharing code between the two implementations,
it could be done by having helper methods in the base class and / or using templates, as you said.

Simon

  reply	other threads:[~2020-03-03 21:31 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200303182057.8973-1-ssbssa.ref@yahoo.de>
2020-03-03 18:21 ` Hannes Domani via gdb-patches
2020-03-03 18:51   ` Eli Zaretskii
2020-03-03 19:12     ` Hannes Domani via gdb-patches
2020-03-03 19:30       ` Eli Zaretskii
2020-03-03 19:36         ` Simon Marchi
2020-03-03 20:28           ` Eli Zaretskii
2020-03-03 21:01             ` Simon Marchi
2020-03-03 21:12               ` Hannes Domani via gdb-patches
2020-03-03 21:31                 ` Simon Marchi [this message]
2020-03-04  3:33               ` Eli Zaretskii
2020-03-13 18:16   ` Tom Tromey

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=5c9e53bd-cbd1-cd59-e701-43a1e76aee3b@simark.ca \
    --to=simark@simark.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=ssbssa@yahoo.de \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).