From: Yao Qi <yao@codesourcery.com>
To: Siddhesh Poyarekar <siddhesh@redhat.com>
Cc: <gdb-patches@sourceware.org>
Subject: Re: [obv][commit] Replace one/two use variables with their values
Date: Mon, 17 Sep 2012 10:33:00 -0000 [thread overview]
Message-ID: <5056FC44.8090207@codesourcery.com> (raw)
In-Reply-To: <20120917143102.467b3fce@spoyarek>
On 09/17/2012 05:01 PM, Siddhesh Poyarekar wrote:
> Hi,
>
> I have committed another obvious patch that replaces variables that are
> used just once or twice, directly with their values.
>
What is your motivation of this change? Does this patch suggest that
we'd better use marcro rather than local variables? I understand most
of it, but ...
> @@ -330,12 +327,11 @@
> if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (TYPE_TARGET_TYPE
> (type)) > 0) {
> elttype = check_typedef (TYPE_TARGET_TYPE (type));
> - eltlen = TYPE_LENGTH (elttype);
> - len = TYPE_LENGTH (type) / eltlen;
> + len = TYPE_LENGTH (type) / TYPE_LENGTH (elttype);
> if (options->prettyprint_arrays)
> print_spaces_filtered (2 + 2 * recurse, stream);
> /* For an array of chars, print with string syntax. */
> - if (eltlen == 1 &&
> + if (TYPE_LENGTH (elttype) == 1 &&
> ((TYPE_CODE (elttype) == TYPE_CODE_INT)
> || ((current_language->la_language == language_m2)
> && (TYPE_CODE (elttype) == TYPE_CODE_CHAR)))
>
... this part. We define a local variable 'eltlen', and use it twice.
Isn't a common programming practise? I don't object to this patch, but
want to know why do we do this change?
> ===================================================================
> RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
> retrieving revision 1.159
> retrieving revision 1.160
> diff -u -r1.159 -r1.160
> --- src/gdb/m68k-tdep.c 2012/07/24 16:37:24 1.159
> +++ src/gdb/m68k-tdep.c 2012/09/17 08:52:18 1.160
> @@ -315,7 +315,6 @@
> m68k_svr4_extract_return_value (struct type *type, struct regcache
> *regcache, gdb_byte *valbuf)
> {
> - int len = TYPE_LENGTH (type);
> gdb_byte buf[M68K_MAX_REGISTER_SIZE];
> struct gdbarch *gdbarch = get_regcache_arch (regcache);
> struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> @@ -326,7 +325,7 @@
> regcache_raw_read (regcache, M68K_FP0_REGNUM, buf);
> convert_typed_floating (buf, fpreg_type, valbuf, type);
> }
> - else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
> + else if (TYPE_CODE (type) == TYPE_CODE_PTR && TYPE_LENGTH (type) ==
> 4) regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
b.t.w, looks your mailer wrap your patch incorrectly.
--
Yao
next prev parent reply other threads:[~2012-09-17 10:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-17 9:02 Siddhesh Poyarekar
2012-09-17 10:33 ` Yao Qi [this message]
2012-09-17 10:46 ` Siddhesh Poyarekar
2012-09-17 11:29 ` Yao Qi
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=5056FC44.8090207@codesourcery.com \
--to=yao@codesourcery.com \
--cc=gdb-patches@sourceware.org \
--cc=siddhesh@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