Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Doug Evans <xdje42@gmail.com>
To: Andy Wingo <wingo@igalia.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 9/9] Remove a useless Guile finalizer
Date: Sat, 12 Apr 2014 20:18:00 -0000	[thread overview]
Message-ID: <m3ob06s3ne.fsf@sspiff.org> (raw)
In-Reply-To: <1397060028-18158-10-git-send-email-wingo@igalia.com> (Andy	Wingo's message of "Wed, 9 Apr 2014 18:13:48 +0200")

Andy Wingo <wingo@igalia.com> writes:

> * gdb/guile/scm-symtab.c (stscm_free_sal_smob): Remove useless free
>   function.  (This was the only useless free function.)
> ---
>  gdb/guile/scm-symtab.c | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/gdb/guile/scm-symtab.c b/gdb/guile/scm-symtab.c
> index 8910973..876bf64 100644
> --- a/gdb/guile/scm-symtab.c
> +++ b/gdb/guile/scm-symtab.c
> @@ -386,19 +386,6 @@ gdbscm_symtab_static_block (SCM self)
>  \f
>  /* Administrivia for sal (symtab-and-line) smobs.  */
>  
> -/* The smob "free" function for <gdb:sal>.  */
> -
> -static size_t
> -stscm_free_sal_smob (SCM self)
> -{
> -  sal_smob *s_smob = (sal_smob *) SCM_SMOB_DATA (self);
> -
> -  /* Not necessary, done to catch bugs.  */
> -  s_smob->symtab_scm = SCM_UNDEFINED;
> -
> -  return 0;
> -}
> -
>  /* The smob "print" function for <gdb:sal>.  */
>  
>  static int
> @@ -692,7 +679,6 @@ gdbscm_initialize_symtabs (void)
>    scm_set_smob_print (symtab_smob_tag, stscm_print_symtab_smob);
>  
>    sal_smob_tag = gdbscm_make_smob_type (sal_smob_name, sizeof (sal_smob));
> -  scm_set_smob_free (sal_smob_tag, stscm_free_sal_smob);
>    scm_set_smob_print (sal_smob_tag, stscm_print_sal_smob);
>  
>    gdbscm_define_functions (symtab_functions, 1);

How useful is valgrind with Guile's GC?
My experience is that the S/N ratio is just too low, but maybe there's
a canonical suppression file that works well enough.
And given that we have this hook, it seems a shame to just throw out
such useful protections against use-after-free (I'm pretty sure early on
I found one bug with them), especially given the subtleties with GC,
and gdb's extensive need to have references to SCM objects from outside
GC-controlled code.

If we're going to have a rule that such code is disallowed,
there is more such code that needs to be removed besides the above
(grep for "catch bugs").

But is this something we want?
At this stage in gdb+guile's development, I like the protection,
and the cost is within epsilon of zero (to me anyway).
[The debug version of the GC may have similar support, but the
benefit of this is that it's "always on" for negligible cost.]
I'm not opposed to removing the code at some later point after things
are really stable and after there's data that they've stopped being
useful.


  reply	other threads:[~2014-04-12 20:18 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-09 16:14 [PATCH 0/8] Cleanups to Guile extension interface Andy Wingo
2014-04-09 16:14 ` [PATCH 2/9] Define and export Guile classes for all GDB object types Andy Wingo
2014-04-12 18:32   ` Doug Evans
2014-04-09 16:14 ` [PATCH 3/9] Fix excess parentheses in Guile extension examples Andy Wingo
2014-04-09 16:14 ` [PATCH 5/9] Rename "gsmob" in Guile interface to "gdb object" Andy Wingo
2014-04-12 18:46   ` Doug Evans
2014-04-17  9:39     ` Andy Wingo
2014-04-17  9:46       ` Eli Zaretskii
2014-04-09 16:14 ` [PATCH 6/9] Remove Guile GDB object property mechanism Andy Wingo
2014-04-12 19:23   ` Doug Evans
2014-04-17  9:54     ` Andy Wingo
2014-04-09 16:14 ` [PATCH 4/9] Fix typos in documentation of Guile `execute' function Andy Wingo
2014-04-09 16:14 ` [PATCH 7/9] Remove Guile mark functions that don't mark anything Andy Wingo
2014-04-12 19:29   ` Doug Evans
2014-04-17  9:48     ` Andy Wingo
2014-04-20 19:34       ` Doug Evans
2014-04-22  7:40         ` Andy Wingo
2014-04-29 11:51         ` Ludovic Courtès
2014-04-09 16:14 ` [PATCH 9/9] Remove a useless Guile finalizer Andy Wingo
2014-04-12 20:18   ` Doug Evans [this message]
2014-04-14 12:12     ` Ludovic Courtès
2014-04-14 21:39       ` Doug Evans
2014-04-15  9:29         ` Ludovic Courtès
2014-04-15 15:30           ` Doug Evans
2014-04-17 10:07     ` Andy Wingo
2014-04-09 16:14 ` [PATCH 8/9] Remove useless Guile SMOB marking functions Andy Wingo
2014-04-12 19:50   ` Doug Evans
2014-04-09 16:14 ` [PATCH 1/9] Allow GDB to build against unreleased Guile 2.2 Andy Wingo
2014-04-12 16:40   ` Doug Evans
2014-04-12 16:47     ` Doug Evans
2014-04-17  9:04     ` Andy Wingo
  -- strict thread matches above, loose matches on Subject: below --
2014-04-09 16:09 [PATCH 0/8] Cleanups to Guile extension interface Andy Wingo
2014-04-09 16:09 ` [PATCH 9/9] Remove a useless Guile finalizer Andy Wingo

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=m3ob06s3ne.fsf@sspiff.org \
    --to=xdje42@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=wingo@igalia.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