Re: [PATCH] Replace some xmalloc-family functions with XNEW-family ones

[Yao Qi <qiyaoltc at gmail dot com>]

Simon Marchi <simon.marchi@ericsson.com> writes:

Hi Simon,
Thanks for doing this.

> This patch is part of the make-gdb-buildable-in-C++ effort.  The idea is
> to change some calls to the xmalloc family of functions to calls to the
> equivalents in the XNEW family.  This avoids adding an explicit cast, so
> it keeps the code a bit more readable.  Some of them also map relatively
> well to a C++ equivalent (XNEW (struct foo) -> new foo), so it will be
> possible to do scripted replacements if needed.

I am not against this patch, and I think this patch is useful to shorten
the code in some cases.  However, I don't think we really need it to
make GDB buildable in C++, right?  We can still use xmalloc in a C++
program (xmalloc is still in use in GCC).

XNEW/xmalloc -> new or struct -> class conversion should be done per
data structure rather than globally like this patch does.  We only
convert C-inheritance data structures like breakpoint_ops, varobj, etc,
to C++ class, and leave the rest there unless it is necessary to change
them to C++ class.  This is my personal understanding, and I'd like to
hear others thoughts.

>
> I only changed calls that were obviously allocating memory for one or
> multiple "objects".  Allocation of variable sizes (such as strings or
> buffer handling) will be for later (and won't use XNEW).
>
>   - xmalloc (sizeof (struct foo)) -> XNEW (struct foo)
>   - xmalloc (num * sizeof (struct foo)) -> XNEWVEC (struct foo, num)
>   - xcalloc (1, sizeof (struct foo)) -> XCNEW (struct foo)
>   - xcalloc (num, sizeof (struct foo)) -> XCNEWVEC (struct foo, num)
>   - xrealloc (p, num * sizeof (struct foo) -> XRESIZEVEC (struct foo, p, num)
>   - obstack_alloc (ob, sizeof (struct foo)) -> XOBNEW (ob, struct foo)
>   - obstack_alloc (ob, num * sizeof (struct foo)) -> XOBNEWVEC (ob, struct foo, num)
>   - alloca (sizeof (struct foo)) -> XALLOCA (struct foo)
>   - alloca (num * sizeof (struct foo)) -> XALLOCAVEC (struct foo, num)
>
> Some instances of xmalloc followed by memset to zero the buffer were
> replaced by XCNEW or XCNEWVEC.

I am not against this conversion.  If we accept this change, are
xmalloc/xcalloc/... not allowed to use, and XNEW/XNEWVEC/... should be
used instead.  The restriction is not necessary to me.

>
> I regtested on x86-64, Ubuntu 14.04, but the patch touches many
> architecture-specific files.  For those I'll have to rely on the
> buildbot or people complaining that I broke their gdb.

If arch-specific files are changes, we have several ways to make sure
changes don't break build at least,

 - Configure gdb with --enable-targets=all --enable-64-bit-bfd and
   build,  all *-tdep.c changes can be covered,
 - Download some cross compile, such as arm/aarch64/mips gcc, and cross
   compile native gdb for these archs, for example, configure gdb with
   --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf CC=/foo/bar/arm-linux-gnueabihf-gcc
 - Install mingw32 toolchain, and cross compile native gdb for win32.

You can also apply for gcc compile farm account to build patched GDB
there.  The complains of buildbot or people are the last guards, and we
should trigger as less complains as we can.

-- 
Yao (éå)