public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Costas Argyris <costas.argyris@gmail.com>
Cc: Jonathan Wakely <jwakely@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] driver: Fix memory leak.
Date: Wed, 6 Dec 2023 15:39:40 +0100	[thread overview]
Message-ID: <ZXCHrFNXe+YvEJX4@tucnak> (raw)
In-Reply-To: <CAHyHGC=CBag7YMOhP4yJfRZcq78BZiVYWLy2SQQKz4CwmzKJLQ@mail.gmail.com>

On Wed, Dec 06, 2023 at 02:29:25PM +0000, Costas Argyris wrote:
> Attached a new patch with these changes.
> 
> On Mon, 4 Dec 2023 at 12:15, Jonathan Wakely <jwakely@redhat.com> wrote:
> 
> > On Sat, 2 Dec 2023 at 21:24, Costas Argyris wrote:
> > >
> > > Use std::vector instead of malloc'd pointer
> > > to get automatic freeing of memory.
> >
> > You can't include <vector> there. Instead you need to define
> > INCLUDE_VECTOR before "system.h"
> >
> > Shouldn't you be using resize, not reserve? Otherwise mdswitches[i] is
> > undefined.

Any reason not to use vec.h instead?
I especially don't like the fact that with a global
std::vector<whatever> var;
it means runtime __cxa_atexit for the var the destruction, which it really
doesn't need on exit.

We really don't need to free the memory at exit time, that is just wasted
cycles, all we need is that it is freed before the pointer or vector is
cleared.

	Jakub


  reply	other threads:[~2023-12-06 14:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-02 21:24 Costas Argyris
2023-12-04 12:15 ` Jonathan Wakely
2023-12-06 14:29   ` Costas Argyris
2023-12-06 14:39     ` Jakub Jelinek [this message]
2023-12-07 14:28       ` Costas Argyris
2023-12-07 14:42         ` Jakub Jelinek
2023-12-07 15:16           ` Costas Argyris
2023-12-07 15:42             ` Jakub Jelinek
2023-12-07 16:01               ` Costas Argyris
2023-12-07 16:04                 ` Jakub Jelinek
2023-12-08 12:18                   ` Costas Argyris
2023-12-08 13:16                     ` Jakub Jelinek

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=ZXCHrFNXe+YvEJX4@tucnak \
    --to=jakub@redhat.com \
    --cc=costas.argyris@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jwakely@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).