From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120353 invoked by alias); 27 Aug 2015 17:48:57 -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 120344 invoked by uid 89); 27 Aug 2015 17:48:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f73.google.com Received: from mail-pa0-f73.google.com (HELO mail-pa0-f73.google.com) (209.85.220.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 27 Aug 2015 17:48:55 +0000 Received: by pabyb7 with SMTP id yb7so3927102pab.0 for ; Thu, 27 Aug 2015 10:48:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc :content-type; bh=JPWcIPIWwuZKoLMX39xtLKzmKxybcZnaRCEokA8MEXM=; b=cZkR3S/hSKm0mx+jHOf+oB4VIVMHqODEIRLNfCwcS9zXxKPgaTO9YvpuXlobxl2wWy 04zy/frRmz9dOegGR7L+plAsT4fyHYW43dpr08vij2SImrZ8MWf0sk5k7sxMjGl12zhA bRNZT8TIO13jA0a6Rj6uYfn8s88SwXp+ZA3FtNUH0dmzZxhye8+RTVY2etGglNxqeIo9 YcZnBNa6BG/RmmHqW+Bv0q7q7V6SLpjieb5wTcuhyp4sxP7GfHy5yIkrKTbOSohx8V6H ewGew2D3Tif/jaBjQWGMoXQtRzMDHYN3B9xRFKeZXIW64NTwE7hjxCCucyfNX3pSRBw4 kRIQ== X-Gm-Message-State: ALoCoQlp7bEzjbhC9hs+DGACwxa+wg8NKLgkdjb2XxwpHkFwO6DSGJxmcmsmolJv3cLGGzPGth84 MIME-Version: 1.0 X-Received: by 10.66.255.42 with SMTP id an10mr5486851pad.40.1440697734045; Thu, 27 Aug 2015 10:48:54 -0700 (PDT) Message-ID: <047d7b15fa29aaec9c051e4e941b@google.com> Date: Thu, 27 Aug 2015 17:48:00 -0000 Subject: Re: [PATCH] Replace some xmalloc-family functions with XNEW-family ones From: Doug Evans To: Simon Marchi Cc: Pedro Alves , Yao Qi , Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00804.txt.bz2 Simon Marchi writes: > On 15-08-26 03:44 PM, Pedro Alves wrote: > > On 08/26/2015 08:24 PM, Simon Marchi wrote: > > > >> I was able to test with just a few targets, those for which I was able to > >> build a toolchain with crosstool-ng (arm and mips). I did mingw32 as well. > > > > Thanks. > > > >> I applied for an access to the gcc compile farm, but I'm still waiting. > >> > >> I don't think I can do better at the moment. Is there still something > >> holding back the patch? > > > > I think you should go ahead. If anything breaks, it should be a trivial fix. > > > > Thanks, > > Pedro Alves > > Alright, pushed a slightly modified (rebased) version: > > > >From 8d7493201cf01c9836403695f67f7e157341bfd5 Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Wed, 26 Aug 2015 17:16:07 -0400 > Subject: [PATCH] Replace some xmalloc-family functions with XNEW-family ones > > 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 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 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. > > gdb/ChangeLog: > > * aarch64-linux-nat.c (aarch64_add_process): Likewise. > * aarch64-tdep.c (aarch64_gdbarch_init): Likewise. > * ada-exp.y (write_ambiguous_var): Likewise. > * ada-lang.c (resolve_subexp): Likewise. > (user_select_syms): Likewise. > (assign_aggregate): Likewise. > (ada_evaluate_subexp): Likewise. > (cache_symbol): Likewise. Hi. First off, thanks for doing this! I don't want to discourage such cleanups by nitpicking. A couple of comments. 1) Apologies you felt you needed to write such a long changelog entry. IMO it's not necessary. There's no doubt a rule that says such verbosity is indeed necessary, but I'd like to think we can bend the rules now and then for cases such as this. I'm not sure what I'd have done differently, I'd have to go through the patch. If all functions in a file were updated I'd be ok with: * ada-lang.c (*): Likewise. There is precedent for doing that. And even if not all functions were updated (and therefore "(*)" is inaccurate), I think a properly worded introductory sentence would make it ok. 2) The changelog entry needs to be made more readable. I'd be ok with just adding a line at the top. E.g., Change some calls to the xmalloc family of functions to calls to the equivalents in the XNEW family. * aarch64-linux-nat.c (aarch64_add_process): Likewise. ... As it currently is all I see is "Likewise." throughout and the reader has no idea what it refers to. [Yeah, the reader can go into the git logs, but they shouldn't have to.]