Fix for PR 10736

Fix for PR 10736

[Sergio Durigan Junior]

Hello guys,

The following patch is a fix for PR 10736.

OK?

-- 
Sergio Durigan Junior
Debugger Engineer
Red Hat Inc.

2010-03-23  Sergio Durigan Junior  <sergiodj@redhat.com>

	PR gdb/10736:
	* xml-syscall.c (my_gdb_datadir): New variable to keep track of
	the changes in data-directory.
	(init_sysinfo): Reload the syscall XML file if the data-directory
	has changed.


diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 3f5585c..2a4d010 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -80,6 +80,10 @@ get_syscall_names (void)
 
 #else /* ! HAVE_LIBEXPAT */
 
+/* Variable that will hold the last known data-directory.  This is useful to
+   know whether we should re-read the XML info for the target.  */
+static char *my_gdb_datadir = NULL;
+
 /* Structure which describes a syscall.  */
 typedef struct syscall_desc
 {
@@ -291,6 +295,18 @@ xml_init_syscalls_info (const char *filename)
 static void
 init_sysinfo (void)
 {
+  /* Should we re-read the XML info for this target?  */
+  if (my_gdb_datadir && strcmp (my_gdb_datadir, gdb_datadir) != 0)
+    {
+      /* The data-directory changed from the last time we used it.
+	 It means that we have to re-read the XML info.  */
+      have_initialized_sysinfo = 0;
+      xfree (my_gdb_datadir);
+      my_gdb_datadir = NULL;
+      if (sysinfo)
+	free_syscalls_info ((void *) sysinfo);
+    }
+
   /* Did we already try to initialize the structure?  */
   if (have_initialized_sysinfo)
     return;
@@ -303,7 +319,8 @@ init_sysinfo (void)
     {
       if (xml_syscall_file)
 	warning (_("\
-Could not load the syscall XML file `%s'."), xml_syscall_file);
+Could not load the syscall XML file `%s/%s'."),
+		 gdb_datadir, xml_syscall_file);
       else
 	warning (_("\
 There is no XML file to open."));
@@ -312,6 +329,9 @@ There is no XML file to open."));
 GDB will not be able to display syscall names nor to verify if\n\
 any provided syscall numbers are valid."));
     }
+
+  /* Saving the data-directory used to read this XML info.  */
+  my_gdb_datadir = xstrdup (gdb_datadir);
 }
 
 static int

Re: Fix for PR 10736

[Sergio Durigan Junior]

On Monday 29 March 2010 21:01:47, Sergio Durigan Junior wrote:
> Hello guys,
> 
> The following patch is a fix for PR 10736.
> 
> OK?

Sorry, I forgot to mention (again!):  regtested on the compile farm, without 
regressions.

-- 
Sergio Durigan Junior
Debugger Engineer
Red Hat Inc.

Re: Fix for PR 10736

[Pedro Alves]

On Tuesday 30 March 2010 01:01:47, Sergio Durigan Junior wrote:
> Hello guys,
> 
> The following patch is a fix for PR 10736.

Thanks!

> OK?

Almost.  There's just a leak to fix.

> +  /* Should we re-read the XML info for this target?  */
> +  if (my_gdb_datadir && strcmp (my_gdb_datadir, gdb_datadir) != 0)
> +    {
> +      /* The data-directory changed from the last time we used it.
> +        It means that we have to re-read the XML info.  */
> +      have_initialized_sysinfo = 0;
> +      xfree (my_gdb_datadir);
> +      my_gdb_datadir = NULL;

The previous data dir is only released if the datadir changed.

> +      if (sysinfo)
> +       free_syscalls_info ((void *) sysinfo);
> +    }

> +
> +  /* Saving the data-directory used to read this XML info.  */
> +  my_gdb_datadir = xstrdup (gdb_datadir);

So here my_gdb_datadir leaks if the datadir _doesn't_ change.
I think you just need to move the xfree and my_data_dir=NULL
statements outside the `if', before the early return.
Anyway, okay with this leak fixed.

-- 
Pedro Alves

Re: Fix for PR 10736

[Sergio Durigan Junior]

Hi Pedro,

On Tuesday 30 March 2010 07:44:25, Pedro Alves wrote:
> > +  /* Should we re-read the XML info for this target?  */
> > +  if (my_gdb_datadir && strcmp (my_gdb_datadir, gdb_datadir) != 0)
> > +    {
> > +      /* The data-directory changed from the last time we used it.
> > +        It means that we have to re-read the XML info.  */
> > +      have_initialized_sysinfo = 0;
> > +      xfree (my_gdb_datadir);
> > +      my_gdb_datadir = NULL;
> 
> The previous data dir is only released if the datadir changed.
> 
> > +      if (sysinfo)
> > +       free_syscalls_info ((void *) sysinfo);
> > +    }
> > 
> > +
> > +  /* Saving the data-directory used to read this XML info.  */
> > +  my_gdb_datadir = xstrdup (gdb_datadir);
> 
> So here my_gdb_datadir leaks if the datadir _doesn't_ change.
> I think you just need to move the xfree and my_data_dir=NULL
> statements outside the `if', before the early return.

Sorry, I don't know if I understood.  If the datadir doesn't change, the call 
to xstrdup will only be made once (i.e., when `have_initialized_sysinfo' is 
zero).  After that, it will never be called again unless datadir changes.

Thanks,

-- 
Sergio Durigan Junior
Debugger Engineer
Red Hat Inc.

Re: Fix for PR 10736

[Pedro Alves]

On Wednesday 31 March 2010 22:14:42, Sergio Durigan Junior wrote:
> Sorry, I don't know if I understood.  If the datadir doesn't change, the call 
> to xstrdup will only be made once (i.e., when `have_initialized_sysinfo' is 
> zero).  After that, it will never be called again unless datadir changes.

Duh on me, sorry.  Patch is okay as is then.

-- 
Pedro Alves

Re: Fix for PR 10736

[Sergio Durigan Junior]

On Wednesday 31 March 2010 21:25:05, Pedro Alves wrote:
> On Wednesday 31 March 2010 22:14:42, Sergio Durigan Junior wrote:
> > Sorry, I don't know if I understood.  If the datadir doesn't change, the
> > call to xstrdup will only be made once (i.e., when
> > `have_initialized_sysinfo' is zero).  After that, it will never be
> > called again unless datadir changes.
> 
> Duh on me, sorry.  Patch is okay as is then.

Sorry for the delay.  Commited now.

Thanks,

-- 
Sergio Durigan Junior
Debugger Engineer
Red Hat Inc.