From: Pedro Alves <palves@redhat.com>
To: "Agovic, Sanimir" <sanimir.agovic@intel.com>
Cc: "'Mark Kettenis'" <mark.kettenis@xs4all.nl>,
"ooprala@redhat.com" <ooprala@redhat.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: C++-compat clean build
Date: Tue, 01 Oct 2013 14:16:00 -0000 [thread overview]
Message-ID: <524AD936.7060604@redhat.com> (raw)
In-Reply-To: <0377C58828D86C4588AEEC42FC3B85A7176850E2@IRSMSX105.ger.corp.intel.com>
On 10/01/2013 01:53 PM, Agovic, Sanimir wrote:
> Thanks Ondrej for taking the initiative!
>
>
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
>> Of Mark Kettenis
>> Sent: Tuesday, October 01, 2013 02:04 PM
>> To: ooprala@redhat.com
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: C++-compat clean build
>>
>> Rejected. Introducing all these casts can't be right. They hide type
>> conversion issues and make the code unreadable.
>>
> Afaik, some casts are required to some extend as C++ does no implicit from/to void *
> conversion. But in general I agree about using casts too vast.
>
> @@ -1598,7 +1598,7 @@ amd64_relocate_instruction (struct gdbarch *gdbarch,
> int len = gdbarch_max_insn_length (gdbarch);
> /* Extra space for sentinels. */
> int fixup_sentinel_space = len;
> - gdb_byte *buf = xmalloc (len + fixup_sentinel_space);
> + gdb_byte *buf = (gdb_byte *) xmalloc (len + fixup_sentinel_space);
> This looks OK to me and could be replaced with a type save new[] once C++ is used.
libiberty gives us a series of macros that kind of mirror
c++ new/new[] (minus the constructors, of course):
libiberty.h:#define XALLOCA(T) ((T *) alloca (sizeof (T)))
...
libiberty.h:#define XNEW(T) ((T *) xmalloc (sizeof (T)))
libiberty.h:#define XCNEW(T) ((T *) xcalloc (1, sizeof (T)))
libiberty.h:#define XNEWVEC(T, N) ((T *) xmalloc (sizeof (T) * (N)))
libiberty.h:#define XCNEWVEC(T, N) ((T *) xcalloc ((N), sizeof (T)))
libiberty.h:#define XNEWVAR(T, S) ((T *) xmalloc ((S)))
libiberty.h:#define XCNEWVAR(T, S) ((T *) xcalloc (1, (S)))
...
etc.
so this case could be written as:
gdb_byte *buf = XNEWVAR (gdb_byte, len + fixup_sentinel_space);
This:
struct foo *f = (struct foo *) xmalloc (sizeof (struct foo));
Can be written as:
struct foo *f = XNEW (struct foo);
There are variants for alloca as well.
We have several uses of these macros already in the tree.
They seem to have the advantage that they hide the casts and perhaps
make the intention of the code clearer while we can't use new/new[].
E.g., XNEW prevents errors of the type:
struct foo *f = (struct foo *) xmalloc (sizeof (f));
We already use these macros in gdb in several places.
Not sure what people feel about using them more?
--
Pedro Alves
next prev parent reply other threads:[~2013-10-01 14:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-01 11:25 Ondrej Oprala
2013-10-01 11:52 ` Ondrej Oprala
2013-10-01 12:04 ` Mark Kettenis
2013-10-01 12:53 ` Agovic, Sanimir
2013-10-01 14:16 ` Pedro Alves [this message]
2013-10-01 16:34 ` Joel Brobecker
2013-10-01 12:53 ` Jan Kratochvil
2013-10-01 12:57 ` Ondrej Oprala
2013-10-01 13:06 ` Tedeschi, Walfred
2013-10-01 13:57 ` Pedro Alves
2013-10-03 20:39 ` 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=524AD936.7060604@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=mark.kettenis@xs4all.nl \
--cc=ooprala@redhat.com \
--cc=sanimir.agovic@intel.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