From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48606 invoked by alias); 18 Aug 2015 08:49:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 48597 invoked by uid 89); 18 Aug 2015 08:49:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f171.google.com Received: from mail-pd0-f171.google.com (HELO mail-pd0-f171.google.com) (209.85.192.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 18 Aug 2015 08:49:11 +0000 Received: by pdob1 with SMTP id b1so10352032pdo.2 for ; Tue, 18 Aug 2015 01:49:09 -0700 (PDT) X-Received: by 10.70.22.234 with SMTP id h10mr11050169pdf.102.1439887749582; Tue, 18 Aug 2015 01:49:09 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id y2sm17282411pdi.80.2015.08.18.01.49.07 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 18 Aug 2015 01:49:08 -0700 (PDT) From: Yao Qi To: Simon Marchi Cc: Subject: Re: [PATCH] Replace some xmalloc-family functions with XNEW-family ones References: <1439848395-1869-1-git-send-email-simon.marchi@ericsson.com> Date: Tue, 18 Aug 2015 08:49:00 -0000 In-Reply-To: <1439848395-1869-1-git-send-email-simon.marchi@ericsson.com> (Simon Marchi's message of "Mon, 17 Aug 2015 17:53:15 -0400") Message-ID: <86614d3rsu.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00453.txt.bz2 Simon Marchi 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, struc= t 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=3Dall --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=3Darm-linux-gnueabihf --target=3Darm-linux-gnueabihf CC=3D/foo/ba= r/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. --=20 Yao (=E9=BD=90=E5=B0=A7)