[RFC] Wording of "catch syscall <number>" warning

[RFC] Wording of "catch syscall <number>" warning

[Doug Evans]

Hi.

The current wording of this warning feels clumsy if syscall names
are unavailable.  It implies there are known syscalls, when there is not.

I'll leave this for a few days and then check it in if there
are no objections.
One alternative is to not print the warning at all if system call
names are unavailable.

2009-09-24  Doug Evans  <dje@google.com>

	* breakpoint.c (catch_syscall_split_args): Use a different warning
	for "catch syscall <number>" when syscall names are unavailable.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.419
diff -u -p -r1.419 breakpoint.c
--- breakpoint.c	15 Sep 2009 03:30:04 -0000	1.419
+++ breakpoint.c	25 Sep 2009 00:22:31 -0000
@@ -7486,9 +7486,16 @@ catch_syscall_split_args (char *arg)
 	    /* We can issue just a warning, but still create the catchpoint.
 	       This is because, even not knowing the syscall name that
 	       this number represents, we can still try to catch the syscall
-	       number.  */
-	    warning (_("The number '%d' does not represent a known syscall."),
-		     syscall_number);
+	       number.  If system call names are unavailable, use a different
+	       wording though.  */
+	    {
+	      if (get_syscall_names () != NULL)
+		warning (_("The number '%d' does not represent a known syscall."),
+			 syscall_number);
+	      else
+		warning (_("Syscall names are unavailable, assuming '%d' is valid."),
+			 syscall_number);
+	    }
 	}
       else
 	{

Re: [RFC] Wording of "catch syscall <number>" warning

[Joel Brobecker]

> One alternative is to not print the warning at all if system call
> names are unavailable.

I'm slightly leaning towards not printing any warning at all. This is
mostly because I dislike warnings when there is nothing I can do about
them.

-- 
Joel

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

Hi Doug,

On Thursday 24 September 2009, Doug Evans wrote:
> The current wording of this warning feels clumsy if syscall names
> are unavailable.  It implies there are known syscalls, when there is not.
> 
> I'll leave this for a few days and then check it in if there
> are no objections.

Thank you for this.  I have one minor comment, though.

> +	      if (get_syscall_names () != NULL)
> +		warning (_("The number '%d' does not represent a known syscall."),
> +			 syscall_number);

I agree with you, there should be warnings covering both cases.  However, this 
patch of yours made me think about performance, specially because you are 
calling get_syscall_names every time you make this check, and I came up with 
another patch.  What do you think about it? 

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 811cdfb..1d35336 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7486,9 +7486,16 @@ catch_syscall_split_args (char *arg)
 	    /* We can issue just a warning, but still create the catchpoint.
 	       This is because, even not knowing the syscall name that
 	       this number represents, we can still try to catch the syscall
-	       number.  */
-	    warning (_("The number '%d' does not represent a known syscall."),
-		     syscall_number);
+	       number.  If system call names are unavailable, use a different
+	       wording though.  */
+	    {
+	      if (syscall_names_available_p ())
+		warning (_("The number '%d' does not represent a known syscall."),
+			 syscall_number);
+	      else
+		warning (_("Syscall names are unavailable, assuming '%d' is valid."),
+			 syscall_number);
+	    }
 	}
       else
 	{
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 15bfe6f..86336b5 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -80,6 +80,11 @@ get_syscall_names (void)
   return NULL;
 }
 
+int
+syscall_names_available_p (void)
+{
+  return 0;
+}
 
 #else /* ! HAVE_LIBEXPAT */
 
@@ -429,4 +434,10 @@ get_syscall_names (void)
   return xml_list_of_syscalls (_sysinfo);
 }
 
+int
+syscall_names_available_p (void)
+{
+  return _sysinfo == NULL ? 0 : 1;
+}
+
 #endif /* ! HAVE_LIBEXPAT */
diff --git a/gdb/xml-syscall.h b/gdb/xml-syscall.h
index 00d3a54..326a856 100644
--- a/gdb/xml-syscall.h
+++ b/gdb/xml-syscall.h
@@ -47,4 +47,8 @@ void get_syscall_by_name (const char *syscall_name, struct 
syscall *s);
 
 const char **get_syscall_names (void);
 
+/* Function used to tell if syscalls names are available.  Returns 1 if
+   they are, 0 otherwise.  */
+int syscall_names_available_p (void);
+
 #endif /* XML_SYSCALL_H */

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Thursday 24 September 2009, Joel Brobecker wrote:
> > One alternative is to not print the warning at all if system call
> > names are unavailable.
> 
> I'm slightly leaning towards not printing any warning at all. This is
> mostly because I dislike warnings when there is nothing I can do about
> them.

I was going to reply Doug's message saying that I'd prefer a warning to be 
printed, but anyway, here is what I think...  I may be misunderstanding things 
here, but I think that warnings are not always intended to ask the user to 
intervent and fix something.  Sometimes, warnings are just intended to tell 
the user "hey, something went wrong while I was working, so you will not be 
able to use feature XYZ".

Of course, this is what I understand by "warning messages", and I will not 
complain if the majority decides to remove them from this piece of code :-).

My two cents.

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Joel Brobecker]

> I was going to reply Doug's message saying that I'd prefer a warning
> to be printed, but anyway, here is what I think...  I may be
> misunderstanding things here, but I think that warnings are not always
> intended to ask the user to intervent and fix something.  Sometimes,
> warnings are just intended to tell the user "hey, something went wrong
> while I was working, so you will not be able to use feature XYZ".

This is really splitting hair, at this point, and I'm happy either way,
but being perfectionist, I'll just explain my reasoning, and let you
guys decide.  In this case, nothing really went "wrong" per se, there
is just a feature that's missing because the person who built the
debugger, which is usually not the same as the user, built the debugger
without expat. If you decide to warn that something went wrong, I'd say
warn only once, something like:

    warning: This debugger was compiled without XML support.
    It will not be able to verify the validity of syscall numbers.

-- 
Joel

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Thursday 24 September 2009, Joel Brobecker wrote:
> > I was going to reply Doug's message saying that I'd prefer a warning
> > to be printed, but anyway, here is what I think...  I may be
> > misunderstanding things here, but I think that warnings are not always
> > intended to ask the user to intervent and fix something.  Sometimes,
> > warnings are just intended to tell the user "hey, something went wrong
> > while I was working, so you will not be able to use feature XYZ".
> 
> This is really splitting hair, at this point, and I'm happy either way,
> but being perfectionist, I'll just explain my reasoning, and let you
> guys decide.  In this case, nothing really went "wrong" per se, there
> is just a feature that's missing because the person who built the
> debugger, which is usually not the same as the user, built the debugger
> without expat. If you decide to warn that something went wrong, I'd say
> warn only once, something like:
> 
>     warning: This debugger was compiled without XML support.
>     It will not be able to verify the validity of syscall numbers.

I see your point.  We already warn the user (only once) if GDB won't be able 
to display syscall names, but we don't tell anything about GDB not being able 
to verify the validity of syscall numbers.  Maybe we should include this 
information in this "one-time warning", and remove the other warnings then.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Doug Evans]

2009/9/24 Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>:
> On Thursday 24 September 2009, Joel Brobecker wrote:
>>     warning: This debugger was compiled without XML support.
>>     It will not be able to verify the validity of syscall numbers.
>
> I see your point.  We already warn the user (only once) if GDB won't be able
> to display syscall names, but we don't tell anything about GDB not being able
> to verify the validity of syscall numbers.  Maybe we should include this
> information in this "one-time warning", and remove the other warnings then.

"works for me", modulo I tend to gloss over the messages gdb prints at
start up. :-)
OTOH, I'm all for keeping the code simple too.

One thing I don't like about my patch is that I'd rather only print
the warning once too.
So how about for now just enhancing the warning at start up, and not
printing anything later on.

Re: [RFC] Wording of "catch syscall <number>" warning

[Joel Brobecker]

> One thing I don't like about my patch is that I'd rather only print
> the warning once too.  So how about for now just enhancing the warning
> at start up, and not printing anything later on.

'works for me.

-- 
Joel

Re: [RFC] Wording of "catch syscall <number>" warning

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

It is so hard to resist commenting on a warning thread.

Doug> One thing I don't like about my patch is that I'd rather only print
Doug> the warning once too.
Doug> So how about for now just enhancing the warning at start up, and not
Doug> printing anything later on.

I had assumed Joel meant to print the warning just the first time the
command is used.  That way users who never use the command will not see
the warning at all.

Tom

Re: [RFC] Wording of "catch syscall <number>" warning

[Tom Tromey]

>>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com> writes:

Sérgio> +  return _sysinfo == NULL ? 0 : 1;

I hadn't noticed this in the syscall patches before, but I think
starting a name with "_" is mildly bad.  Those names are reserved for
the system.

Tom

Re: [RFC] Wording of "catch syscall <number>" warning

[Joel Brobecker]

> I had assumed Joel meant to print the warning just the first time the
> command is used.  That way users who never use the command will not see
> the warning at all.

Good suggestion!

-- 
Joel

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Friday 25 September 2009, Tom Tromey wrote:
> >>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>
> >>>>> writes:
> 
> Sérgio> +  return _sysinfo == NULL ? 0 : 1;
> 
> I hadn't noticed this in the syscall patches before, but I think
> starting a name with "_" is mildly bad.  Those names are reserved for
> the system.

I will fix this ASAP.  Thank you, Tom.

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Friday 25 September 2009, Joel Brobecker wrote:
> > I had assumed Joel meant to print the warning just the first time the
> > command is used.  That way users who never use the command will not see
> > the warning at all.
> 
> Good suggestion!

Actually, that's what I understood from the beginning :-).  Anyway, I will 
send a patch fixing this.

Thank you.

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Friday 25 September 2009, Tom Tromey wrote:
> >>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>
> >>>>> writes:
> 
> Sérgio> +  return _sysinfo == NULL ? 0 : 1;
> 
> I hadn't noticed this in the syscall patches before, but I think
> starting a name with "_" is mildly bad.  Those names are reserved for
> the system.

I addressed all the issues.  Please, take a look at this patch.

Thank you,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 15bfe6f..9ac0947 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -114,7 +114,7 @@ struct syscall_parsing_data
 
 /* Structure used to store information about the available syscalls in
    the system.  */
-static const struct syscalls_info *_sysinfo = NULL;
+static const struct syscalls_info *sysinfo = NULL;
 
 /* A flag to tell if we already initialized the structure above.  */
 static int have_initialized_sysinfo = 0;
@@ -275,10 +275,7 @@ xml_init_syscalls_info (const char *filename)
 
   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
   if (full_file == NULL)
-    {
-      warning (_("Could not open \"%s\""), filename);
-      return NULL;
-    }
+    return NULL;
 
   back_to = make_cleanup (xfree, full_file);
 
@@ -300,27 +297,23 @@ init_sysinfo (void)
   /* Did we already try to initialize the structure?  */
   if (have_initialized_sysinfo)
     return;
-/*  if (xml_syscall_file == NULL)
-    internal_error (__FILE__, __LINE__,
-                    _("This architecture has not set the XML syscall file "
-                      "name.  This is a bug and should not happen; please "
-                      "report it.")); */
 
-  _sysinfo = xml_init_syscalls_info (xml_syscall_file);
+  sysinfo = xml_init_syscalls_info (xml_syscall_file);
 
   have_initialized_sysinfo = 1;
 
-  if (_sysinfo == NULL)
+  if (sysinfo == NULL)
     {
       if (xml_syscall_file)
-        /* The initialization failed.  Let's show a warning
-           message to the user (just this time) and leave.  */
-        warning (_("Could not load the syscall XML file `%s'.\n\
-GDB will not be able to display syscall names."), xml_syscall_file);
+	warning (_("\
+Could not load the syscall XML file `%s'."), xml_syscall_file);
       else
-        /* There's no file to open.  Let's warn the user.  */
-        warning (_("There is no XML file to open.\n\
-GDB will not be able to display syscall names."));
+	warning (_("\
+There is no XML file to open."));
+
+      warning (_("\
+GDB will not be able to display syscall names nor to verify if\n\
+any provided syscall numbers are valid."));
     }
 }
 
@@ -408,7 +401,7 @@ get_syscall_by_number (int syscall_number,
   init_sysinfo ();
 
   s->number = syscall_number;
-  s->name = xml_get_syscall_name (_sysinfo, syscall_number);
+  s->name = xml_get_syscall_name (sysinfo, syscall_number);
 }
 
 void
@@ -417,7 +410,7 @@ get_syscall_by_name (const char *syscall_name,
 {
   init_sysinfo ();
 
-  s->number = xml_get_syscall_number (_sysinfo, syscall_name);
+  s->number = xml_get_syscall_number (sysinfo, syscall_name);
   s->name = syscall_name;
 }
 
@@ -426,7 +419,7 @@ get_syscall_names (void)
 {
   init_sysinfo ();
 
-  return xml_list_of_syscalls (_sysinfo);
+  return xml_list_of_syscalls (sysinfo);
 }
 
 #endif /* ! HAVE_LIBEXPAT */

Re: [RFC] Wording of "catch syscall <number>" warning

[Doug Evans]

2009/9/25 Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>:
> On Friday 25 September 2009, Tom Tromey wrote:
>> >>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>
>> >>>>> writes:
>>
>> Sérgio> +  return _sysinfo == NULL ? 0 : 1;
>>
>> I hadn't noticed this in the syscall patches before, but I think
>> starting a name with "_" is mildly bad.  Those names are reserved for
>> the system.
>
> I addressed all the issues.  Please, take a look at this patch.
>
> Thank you,
>
> --
> Sérgio Durigan Júnior
> Linux on Power Toolchain - Software Engineer
> Linux Technology Center - LTC
> IBM Brazil
>
> diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
> index 15bfe6f..9ac0947 100644
> --- a/gdb/xml-syscall.c
> +++ b/gdb/xml-syscall.c
> @@ -114,7 +114,7 @@ struct syscall_parsing_data
>
>  /* Structure used to store information about the available syscalls in
>    the system.  */
> -static const struct syscalls_info *_sysinfo = NULL;
> +static const struct syscalls_info *sysinfo = NULL;
>
>  /* A flag to tell if we already initialized the structure above.  */
>  static int have_initialized_sysinfo = 0;
> @@ -275,10 +275,7 @@ xml_init_syscalls_info (const char *filename)
>
>   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
>   if (full_file == NULL)
> -    {
> -      warning (_("Could not open \"%s\""), filename);
> -      return NULL;
> -    }
> +    return NULL;
>
>   back_to = make_cleanup (xfree, full_file);
>
> @@ -300,27 +297,23 @@ init_sysinfo (void)
>   /* Did we already try to initialize the structure?  */
>   if (have_initialized_sysinfo)
>     return;
> -/*  if (xml_syscall_file == NULL)
> -    internal_error (__FILE__, __LINE__,
> -                    _("This architecture has not set the XML syscall file "
> -                      "name.  This is a bug and should not happen; please "
> -                      "report it.")); */
>
> -  _sysinfo = xml_init_syscalls_info (xml_syscall_file);
> +  sysinfo = xml_init_syscalls_info (xml_syscall_file);
>
>   have_initialized_sysinfo = 1;
>
> -  if (_sysinfo == NULL)
> +  if (sysinfo == NULL)
>     {
>       if (xml_syscall_file)
> -        /* The initialization failed.  Let's show a warning
> -           message to the user (just this time) and leave.  */
> -        warning (_("Could not load the syscall XML file `%s'.\n\
> -GDB will not be able to display syscall names."), xml_syscall_file);
> +       warning (_("\
> +Could not load the syscall XML file `%s'."), xml_syscall_file);
>       else
> -        /* There's no file to open.  Let's warn the user.  */
> -        warning (_("There is no XML file to open.\n\
> -GDB will not be able to display syscall names."));
> +       warning (_("\
> +There is no XML file to open."));
> +
> +      warning (_("\
> +GDB will not be able to display syscall names nor to verify if\n\
> +any provided syscall numbers are valid."));
>     }
>  }
>
> @@ -408,7 +401,7 @@ get_syscall_by_number (int syscall_number,
>   init_sysinfo ();
>
>   s->number = syscall_number;
> -  s->name = xml_get_syscall_name (_sysinfo, syscall_number);
> +  s->name = xml_get_syscall_name (sysinfo, syscall_number);
>  }
>
>  void
> @@ -417,7 +410,7 @@ get_syscall_by_name (const char *syscall_name,
>  {
>   init_sysinfo ();
>
> -  s->number = xml_get_syscall_number (_sysinfo, syscall_name);
> +  s->number = xml_get_syscall_number (sysinfo, syscall_name);
>   s->name = syscall_name;
>  }
>
> @@ -426,7 +419,7 @@ get_syscall_names (void)
>  {
>   init_sysinfo ();
>
> -  return xml_list_of_syscalls (_sysinfo);
> +  return xml_list_of_syscalls (sysinfo);
>  }
>
>  #endif /* ! HAVE_LIBEXPAT */
>

It wasn't clear that this addressed my concerns when !HAVE_LIBEXPAT so
I applied the patch and gave it a spin.
Two nits:
I still see a warning at start-up, and
When I do "catch syscall" I still get
warning: The number '20' does not represent a known syscall.
for every invocation. [Did I misunderstand?  Or did we want this
warning, which is issued in breakpoint.c, to only happen once if
!HAVE_LIBEXPAT.]

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Friday 25 September 2009, Sérgio Durigan Júnior wrote:
> On Friday 25 September 2009, Tom Tromey wrote:
> > >>>>> "Sérgio" == Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>
> > >>>>> writes:
> >
> > Sérgio> +  return _sysinfo == NULL ? 0 : 1;
> >
> > I hadn't noticed this in the syscall patches before, but I think
> > starting a name with "_" is mildly bad.  Those names are reserved for
> > the system.
> 
> I addressed all the issues.  Please, take a look at this patch.

Actually, not all of them.  Sorry, I forgot to remove the extra warning from 
breakpoint.c.  Now the patch is complete.

Thanks,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 811cdfb..da05eb3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7479,17 +7479,7 @@ catch_syscall_split_args (char *arg)
       /* Check if the user provided a syscall name or a number.  */
       syscall_number = (int) strtol (cur_name, &endptr, 0);
       if (*endptr == '\0')
-	{
-	  get_syscall_by_number (syscall_number, &s);
-
-	  if (s.name == NULL)
-	    /* We can issue just a warning, but still create the catchpoint.
-	       This is because, even not knowing the syscall name that
-	       this number represents, we can still try to catch the syscall
-	       number.  */
-	    warning (_("The number '%d' does not represent a known syscall."),
-		     syscall_number);
-	}
+	get_syscall_by_number (syscall_number, &s);
       else
 	{
 	  /* We have a name.  Let's check if it's valid and convert it
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 15bfe6f..9ac0947 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -114,7 +114,7 @@ struct syscall_parsing_data
 
 /* Structure used to store information about the available syscalls in
    the system.  */
-static const struct syscalls_info *_sysinfo = NULL;
+static const struct syscalls_info *sysinfo = NULL;
 
 /* A flag to tell if we already initialized the structure above.  */
 static int have_initialized_sysinfo = 0;
@@ -275,10 +275,7 @@ xml_init_syscalls_info (const char *filename)
 
   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
   if (full_file == NULL)
-    {
-      warning (_("Could not open \"%s\""), filename);
-      return NULL;
-    }
+    return NULL;
 
   back_to = make_cleanup (xfree, full_file);
 
@@ -300,27 +297,23 @@ init_sysinfo (void)
   /* Did we already try to initialize the structure?  */
   if (have_initialized_sysinfo)
     return;
-/*  if (xml_syscall_file == NULL)
-    internal_error (__FILE__, __LINE__,
-                    _("This architecture has not set the XML syscall file "
-                      "name.  This is a bug and should not happen; please "
-                      "report it.")); */
 
-  _sysinfo = xml_init_syscalls_info (xml_syscall_file);
+  sysinfo = xml_init_syscalls_info (xml_syscall_file);
 
   have_initialized_sysinfo = 1;
 
-  if (_sysinfo == NULL)
+  if (sysinfo == NULL)
     {
       if (xml_syscall_file)
-        /* The initialization failed.  Let's show a warning
-           message to the user (just this time) and leave.  */
-        warning (_("Could not load the syscall XML file `%s'.\n\
-GDB will not be able to display syscall names."), xml_syscall_file);
+	warning (_("\
+Could not load the syscall XML file `%s'."), xml_syscall_file);
       else
-        /* There's no file to open.  Let's warn the user.  */
-        warning (_("There is no XML file to open.\n\
-GDB will not be able to display syscall names."));
+	warning (_("\
+There is no XML file to open."));
+
+      warning (_("\
+GDB will not be able to display syscall names nor to verify if\n\
+any provided syscall numbers are valid."));
     }
 }
 
@@ -408,7 +401,7 @@ get_syscall_by_number (int syscall_number,
   init_sysinfo ();
 
   s->number = syscall_number;
-  s->name = xml_get_syscall_name (_sysinfo, syscall_number);
+  s->name = xml_get_syscall_name (sysinfo, syscall_number);
 }
 
 void
@@ -417,7 +410,7 @@ get_syscall_by_name (const char *syscall_name,
 {
   init_sysinfo ();
 
-  s->number = xml_get_syscall_number (_sysinfo, syscall_name);
+  s->number = xml_get_syscall_number (sysinfo, syscall_name);
   s->name = syscall_name;
 }
 
@@ -426,7 +419,7 @@ get_syscall_names (void)
 {
   init_sysinfo ();
 
-  return xml_list_of_syscalls (_sysinfo);
+  return xml_list_of_syscalls (sysinfo);
 }
 
 #endif /* ! HAVE_LIBEXPAT */

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

Hi Doug,

On Friday 25 September 2009, Doug Evans wrote:
> It wasn't clear that this addressed my concerns when !HAVE_LIBEXPAT so
> I applied the patch and gave it a spin.

[I am assuming that you are talking about our little discussion on IRC, about 
fix the testsuite so that it doesn't fail when the user doesn't have 
libexpat.]

You are right, this patch does not address your concerns about !HAVE_LIBEXPAT, 
mainly because we discussed that on IRC _after_ I sent this patch :-).  I will 
resubmit another version that addresses this issue as well.

> Two nits:
> I still see a warning at start-up, and
> When I do "catch syscall" I still get
> warning: The number '20' does not represent a known syscall.
> for every invocation. [Did I misunderstand?  Or did we want this
> warning, which is issued in breakpoint.c, to only happen once if
> !HAVE_LIBEXPAT.]

That's because you took the wrong version of the patch :-).  Please, take a 
look at my other message following this one.

http://sourceware.org/ml/gdb-patches/2009-09/msg00826.html

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Friday 25 September 2009, Doug Evans wrote:
> It wasn't clear that this addressed my concerns when !HAVE_LIBEXPAT so
> I applied the patch and gave it a spin.
> Two nits:
> I still see a warning at start-up, and
> When I do "catch syscall" I still get
> warning: The number '20' does not represent a known syscall.
> for every invocation. [Did I misunderstand?  Or did we want this
> warning, which is issued in breakpoint.c, to only happen once if
> !HAVE_LIBEXPAT.]

Ok, so here it is.  This patch updates the testcase so that it doesn't fail 
when GDB is built without expat support.  Also, it removes the unecessary 
warning from breakpoint.c, and updates that existing warning in xml-syscall.c 
in order to tell the user that GDB won't be able to verify syscall numbers if 
no XML support is present.

What do you think now?

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

gdb/ChangeLog:

2009-26-09  Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>

	* breakpoint.c (catch_syscall_split_args): Remove unecessary warning
	displayed when the user provided a syscall name and there is no
	XML support.
	* xml-syscall.c: Renamed `_sysinfo' to `sysinfo'.
	(set_xml_syscall_file_name): Remove syscall_warn_user.
	(xml_init_syscalls_info): Remove warning.
	(init_sysinfo): Update warnings.

gdb/testsuite/ChangeLog:

2009-26-09  Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>

	* gdb.base/catch-syscall.exp: Adapt the testcase in order to accept
	the modified warnings for catch syscall.  Verify if GDB was compiled
	with support for lib expat, and choose which tests to run depending
	on this.


diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 811cdfb..da05eb3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7479,17 +7479,7 @@ catch_syscall_split_args (char *arg)
       /* Check if the user provided a syscall name or a number.  */
       syscall_number = (int) strtol (cur_name, &endptr, 0);
       if (*endptr == '\0')
-	{
-	  get_syscall_by_number (syscall_number, &s);
-
-	  if (s.name == NULL)
-	    /* We can issue just a warning, but still create the catchpoint.
-	       This is because, even not knowing the syscall name that
-	       this number represents, we can still try to catch the syscall
-	       number.  */
-	    warning (_("The number '%d' does not represent a known syscall."),
-		     syscall_number);
-	}
+	get_syscall_by_number (syscall_number, &s);
       else
 	{
 	  /* We have a name.  Let's check if it's valid and convert it
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp 
b/gdb/testsuite/gdb.base/catch-syscall.exp
index 6798d04..b68aa8a 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -252,6 +252,25 @@ proc test_catch_syscall_restarting_inferior {} {
     check_for_program_end
 }
 
+proc test_catch_syscall_fail_nodatadir {} {
+    global gdb_prompt
+
+    # Sanitizing.
+    delete_breakpoints
+
+    # Testing to see if we receive a warning when calling "catch syscall"
+    # without XML support (without datadir).
+    set thistest "Catch syscall displays a warning when there is no XML 
support (no datadir set)"
+    gdb_test "catch syscall" "warning: Could not load the syscall XML 
file.*warning: GDB will not be able to display syscall names nor to verify 
if.*any provided syscall numbers are valid.*Catchpoint .*(syscall).*" 
$thistest
+
+    # Since the catchpoint was set, we must check if it's present at
+    # "info breakpoints"
+    check_info_bp_any_syscall
+
+    # Sanitizing.
+    delete_breakpoints
+}
+
 proc do_syscall_tests {} {
     global gdb_prompt srcdir
 
@@ -295,25 +314,6 @@ proc do_syscall_tests {} {
     if [runto_main] then { test_catch_syscall_restarting_inferior }
 }
 
-proc test_catch_syscall_fail_noxml {} {
-    global gdb_prompt
-
-    # Sanitizing.
-    delete_breakpoints
-
-    # Testing to see if we receive a warning when calling "catch syscall"
-    # without XML support.
-    set thistest "Catch syscall displays a warning when there is no XML 
support"
-    gdb_test "catch syscall" "warning: Could not open .*warning: Could not 
load the syscall XML file .*GDB will not be able to display syscall 
names.*Catchpoint .*(syscall).*" $thistest
-
-    # Since the catchpoint was set, we must check if it's present at
-    # "info breakpoints"
-    check_info_bp_any_syscall
-
-    # Sanitizing.
-    delete_breakpoints
-}
-
 proc test_catch_syscall_without_args_noxml {} {
     # We will need the syscall names even not using it
     # because we need to know know many syscalls are in
@@ -386,11 +386,7 @@ proc do_syscall_tests_without_xml {} {
     # we want GDB to display only numbers, not names.  So, let's
     # begin with the tests.
 
-    # The first test is to see if GDB displays a warning when we
-    # try to catch syscalls without the XML support.
-    test_catch_syscall_fail_noxml
-
-    # Now, let's test if we can catch syscalls without XML support.
+    # Let's test if we can catch syscalls without XML support.
     # We should succeed, but GDB is not supposed to print syscall names.
     if [runto_main] then { test_catch_syscall_without_args_noxml }
 
@@ -426,12 +422,24 @@ proc fill_all_syscalls_numbers {} {
 # Start with a fresh gdb
 
 gdb_exit
+set do_xml_test ![gdb_skip_xml_test]
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
 # Execute the tests, using XML support
-do_syscall_tests
+if $do_xml_test {
+  do_syscall_tests
+
+  # Now, we have to see if GDB displays a warning when we
+  # don't set the data-directory but try to use catch syscall
+  # anyway.  For that, we must restart GDB first.
+  gdb_exit
+  gdb_start
+  gdb_reinitialize_dir $srcdir/$subdir
+  gdb_load ${binfile}
+  test_catch_syscall_fail_nodatadir
+}
 
 # Restart gdb
 
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 15bfe6f..963ebcd 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -1,9 +1,7 @@
 /* Functions that provide the mechanism to parse a syscall XML file
    and get its values.
 
-   Copyright (C) 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996,
-   1998, 1999, 2000, 2001, 2003, 2004, 2005, 2006, 2007, 2008
-   Free Software Foundation, Inc.
+   Copyright (C) 2009  Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -52,7 +50,7 @@ syscall_warn_user (void)
 void
 set_xml_syscall_file_name (const char *name)
 {
-  syscall_warn_user ();
+  return;
 }
 
 void
@@ -80,7 +78,6 @@ get_syscall_names (void)
   return NULL;
 }
 
-
 #else /* ! HAVE_LIBEXPAT */
 
 /* Structure which describes a syscall.  */
@@ -114,7 +111,7 @@ struct syscall_parsing_data
 
 /* Structure used to store information about the available syscalls in
    the system.  */
-static const struct syscalls_info *_sysinfo = NULL;
+static const struct syscalls_info *sysinfo = NULL;
 
 /* A flag to tell if we already initialized the structure above.  */
 static int have_initialized_sysinfo = 0;
@@ -275,10 +272,7 @@ xml_init_syscalls_info (const char *filename)
 
   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
   if (full_file == NULL)
-    {
-      warning (_("Could not open \"%s\""), filename);
-      return NULL;
-    }
+    return NULL;
 
   back_to = make_cleanup (xfree, full_file);
 
@@ -300,27 +294,23 @@ init_sysinfo (void)
   /* Did we already try to initialize the structure?  */
   if (have_initialized_sysinfo)
     return;
-/*  if (xml_syscall_file == NULL)
-    internal_error (__FILE__, __LINE__,
-                    _("This architecture has not set the XML syscall file "
-                      "name.  This is a bug and should not happen; please "
-                      "report it.")); */
 
-  _sysinfo = xml_init_syscalls_info (xml_syscall_file);
+  sysinfo = xml_init_syscalls_info (xml_syscall_file);
 
   have_initialized_sysinfo = 1;
 
-  if (_sysinfo == NULL)
+  if (sysinfo == NULL)
     {
       if (xml_syscall_file)
-        /* The initialization failed.  Let's show a warning
-           message to the user (just this time) and leave.  */
-        warning (_("Could not load the syscall XML file `%s'.\n\
-GDB will not be able to display syscall names."), xml_syscall_file);
+	warning (_("\
+Could not load the syscall XML file `%s'."), xml_syscall_file);
       else
-        /* There's no file to open.  Let's warn the user.  */
-        warning (_("There is no XML file to open.\n\
-GDB will not be able to display syscall names."));
+	warning (_("\
+There is no XML file to open."));
+
+      warning (_("\
+GDB will not be able to display syscall names nor to verify if\n\
+any provided syscall numbers are valid."));
     }
 }
 
@@ -408,7 +398,7 @@ get_syscall_by_number (int syscall_number,
   init_sysinfo ();
 
   s->number = syscall_number;
-  s->name = xml_get_syscall_name (_sysinfo, syscall_number);
+  s->name = xml_get_syscall_name (sysinfo, syscall_number);
 }
 
 void
@@ -417,7 +407,7 @@ get_syscall_by_name (const char *syscall_name,
 {
   init_sysinfo ();
 
-  s->number = xml_get_syscall_number (_sysinfo, syscall_name);
+  s->number = xml_get_syscall_number (sysinfo, syscall_name);
   s->name = syscall_name;
 }
 
@@ -426,7 +416,7 @@ get_syscall_names (void)
 {
   init_sysinfo ();
 
-  return xml_list_of_syscalls (_sysinfo);
+  return xml_list_of_syscalls (sysinfo);
 }
 
 #endif /* ! HAVE_LIBEXPAT */

Re: [RFC] Wording of "catch syscall <number>" warning

[Doug Evans]

2009/9/26 Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>:
> Ok, so here it is.  This patch updates the testcase so that it doesn't fail
> when GDB is built without expat support.  Also, it removes the unecessary
> warning from breakpoint.c, and updates that existing warning in xml-syscall.c
> in order to tell the user that GDB won't be able to verify syscall numbers if
> no XML support is present.
>
> What do you think now?

Thanks.
It's fine with me.

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Monday 28 September 2009, Doug Evans wrote:
> 2009/9/26 Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>:
> > Ok, so here it is.  This patch updates the testcase so that it doesn't
> > fail when GDB is built without expat support.  Also, it removes the
> > unecessary warning from breakpoint.c, and updates that existing warning
> > in xml-syscall.c in order to tell the user that GDB won't be able to
> > verify syscall numbers if no XML support is present.
> >
> > What do you think now?
> 
> Thanks.
> It's fine with me.

I'm sorry about the (long) delay...  So, is it still OK to check in?

Thanks,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

Re: [RFC] Wording of "catch syscall <number>" warning

[Sérgio Durigan Júnior]

On Saturday 03 October 2009, Sérgio Durigan Júnior wrote:
> On Monday 28 September 2009, Doug Evans wrote:
> > 2009/9/26 Sérgio Durigan Júnior <sergiodj@linux.vnet.ibm.com>:
> > > Ok, so here it is.  This patch updates the testcase so that it doesn't
> > > fail when GDB is built without expat support.  Also, it removes the
> > > unecessary warning from breakpoint.c, and updates that existing warning
> > > in xml-syscall.c in order to tell the user that GDB won't be able to
> > > verify syscall numbers if no XML support is present.
> > >
> > > What do you think now?
> >
> > Thanks.
> > It's fine with me.

I've checked in this patch.  If there is any problem, please tell me.

Regards,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

gdb/ChangeLog:

2009-31-10  Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>

	* breakpoint.c (catch_syscall_split_args): Remove unecessary warning
	displayed when the user provided a syscall name and there is no
	XML support.
	* xml-syscall.c: Renamed `_sysinfo' to `sysinfo'.
	(set_xml_syscall_file_name): Remove syscall_warn_user.
	(xml_init_syscalls_info): Remove warning.
	(init_sysinfo): Update warnings.

gdb/testsuite/ChangeLog:

2009-31-10  Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>

	* gdb.base/catch-syscall.exp: Adapt the testcase in order to accept
	the modified warnings for catch syscall.  Verify if GDB was compiled
	with support for lib expat, and choose which tests to run depending
	on this.


diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index cf11f51..6bf9262 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7804,17 +7804,7 @@ catch_syscall_split_args (char *arg)
       /* Check if the user provided a syscall name or a number.  */
       syscall_number = (int) strtol (cur_name, &endptr, 0);
       if (*endptr == '\0')
-	{
-	  get_syscall_by_number (syscall_number, &s);
-
-	  if (s.name == NULL)
-	    /* We can issue just a warning, but still create the catchpoint.
-	       This is because, even not knowing the syscall name that
-	       this number represents, we can still try to catch the syscall
-	       number.  */
-	    warning (_("The number '%d' does not represent a known syscall."),
-		     syscall_number);
-	}
+	get_syscall_by_number (syscall_number, &s);
       else
 	{
 	  /* We have a name.  Let's check if it's valid and convert it
diff --git a/gdb/testsuite/gdb.base/catch-syscall.exp 
b/gdb/testsuite/gdb.base/catch-syscall.exp
index 6798d04..b68aa8a 100644
--- a/gdb/testsuite/gdb.base/catch-syscall.exp
+++ b/gdb/testsuite/gdb.base/catch-syscall.exp
@@ -252,6 +252,25 @@ proc test_catch_syscall_restarting_inferior {} {
     check_for_program_end
 }
 
+proc test_catch_syscall_fail_nodatadir {} {
+    global gdb_prompt
+
+    # Sanitizing.
+    delete_breakpoints
+
+    # Testing to see if we receive a warning when calling "catch syscall"
+    # without XML support (without datadir).
+    set thistest "Catch syscall displays a warning when there is no XML 
support (no datadir set)"
+    gdb_test "catch syscall" "warning: Could not load the syscall XML 
file.*warning: GDB will not be able to display syscall names nor to verify 
if.*any provided syscall numbers are valid.*Catchpoint .*(syscall).*" 
$thistest
+
+    # Since the catchpoint was set, we must check if it's present at
+    # "info breakpoints"
+    check_info_bp_any_syscall
+
+    # Sanitizing.
+    delete_breakpoints
+}
+
 proc do_syscall_tests {} {
     global gdb_prompt srcdir
 
@@ -295,25 +314,6 @@ proc do_syscall_tests {} {
     if [runto_main] then { test_catch_syscall_restarting_inferior }
 }
 
-proc test_catch_syscall_fail_noxml {} {
-    global gdb_prompt
-
-    # Sanitizing.
-    delete_breakpoints
-
-    # Testing to see if we receive a warning when calling "catch syscall"
-    # without XML support.
-    set thistest "Catch syscall displays a warning when there is no XML 
support"
-    gdb_test "catch syscall" "warning: Could not open .*warning: Could not 
load the syscall XML file .*GDB will not be able to display syscall 
names.*Catchpoint .*(syscall).*" $thistest
-
-    # Since the catchpoint was set, we must check if it's present at
-    # "info breakpoints"
-    check_info_bp_any_syscall
-
-    # Sanitizing.
-    delete_breakpoints
-}
-
 proc test_catch_syscall_without_args_noxml {} {
     # We will need the syscall names even not using it
     # because we need to know know many syscalls are in
@@ -386,11 +386,7 @@ proc do_syscall_tests_without_xml {} {
     # we want GDB to display only numbers, not names.  So, let's
     # begin with the tests.
 
-    # The first test is to see if GDB displays a warning when we
-    # try to catch syscalls without the XML support.
-    test_catch_syscall_fail_noxml
-
-    # Now, let's test if we can catch syscalls without XML support.
+    # Let's test if we can catch syscalls without XML support.
     # We should succeed, but GDB is not supposed to print syscall names.
     if [runto_main] then { test_catch_syscall_without_args_noxml }
 
@@ -426,12 +422,24 @@ proc fill_all_syscalls_numbers {} {
 # Start with a fresh gdb
 
 gdb_exit
+set do_xml_test ![gdb_skip_xml_test]
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
 # Execute the tests, using XML support
-do_syscall_tests
+if $do_xml_test {
+  do_syscall_tests
+
+  # Now, we have to see if GDB displays a warning when we
+  # don't set the data-directory but try to use catch syscall
+  # anyway.  For that, we must restart GDB first.
+  gdb_exit
+  gdb_start
+  gdb_reinitialize_dir $srcdir/$subdir
+  gdb_load ${binfile}
+  test_catch_syscall_fail_nodatadir
+}
 
 # Restart gdb
 
diff --git a/gdb/xml-syscall.c b/gdb/xml-syscall.c
index 15bfe6f..963ebcd 100644
--- a/gdb/xml-syscall.c
+++ b/gdb/xml-syscall.c
@@ -1,9 +1,7 @@
 /* Functions that provide the mechanism to parse a syscall XML file
    and get its values.
 
-   Copyright (C) 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996,
-   1998, 1999, 2000, 2001, 2003, 2004, 2005, 2006, 2007, 2008
-   Free Software Foundation, Inc.
+   Copyright (C) 2009  Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -52,7 +50,7 @@ syscall_warn_user (void)
 void
 set_xml_syscall_file_name (const char *name)
 {
-  syscall_warn_user ();
+  return;
 }
 
 void
@@ -80,7 +78,6 @@ get_syscall_names (void)
   return NULL;
 }
 
-
 #else /* ! HAVE_LIBEXPAT */
 
 /* Structure which describes a syscall.  */
@@ -114,7 +111,7 @@ struct syscall_parsing_data
 
 /* Structure used to store information about the available syscalls in
    the system.  */
-static const struct syscalls_info *_sysinfo = NULL;
+static const struct syscalls_info *sysinfo = NULL;
 
 /* A flag to tell if we already initialized the structure above.  */
 static int have_initialized_sysinfo = 0;
@@ -275,10 +272,7 @@ xml_init_syscalls_info (const char *filename)
 
   full_file = xml_fetch_content_from_file (filename, gdb_datadir);
   if (full_file == NULL)
-    {
-      warning (_("Could not open \"%s\""), filename);
-      return NULL;
-    }
+    return NULL;
 
   back_to = make_cleanup (xfree, full_file);
 
@@ -300,27 +294,23 @@ init_sysinfo (void)
   /* Did we already try to initialize the structure?  */
   if (have_initialized_sysinfo)
     return;
-/*  if (xml_syscall_file == NULL)
-    internal_error (__FILE__, __LINE__,
-                    _("This architecture has not set the XML syscall file "
-                      "name.  This is a bug and should not happen; please "
-                      "report it.")); */
 
-  _sysinfo = xml_init_syscalls_info (xml_syscall_file);
+  sysinfo = xml_init_syscalls_info (xml_syscall_file);
 
   have_initialized_sysinfo = 1;
 
-  if (_sysinfo == NULL)
+  if (sysinfo == NULL)
     {
       if (xml_syscall_file)
-        /* The initialization failed.  Let's show a warning
-           message to the user (just this time) and leave.  */
-        warning (_("Could not load the syscall XML file `%s'.\n\
-GDB will not be able to display syscall names."), xml_syscall_file);
+	warning (_("\
+Could not load the syscall XML file `%s'."), xml_syscall_file);
       else
-        /* There's no file to open.  Let's warn the user.  */
-        warning (_("There is no XML file to open.\n\
-GDB will not be able to display syscall names."));
+	warning (_("\
+There is no XML file to open."));
+
+      warning (_("\
+GDB will not be able to display syscall names nor to verify if\n\
+any provided syscall numbers are valid."));
     }
 }
 
@@ -408,7 +398,7 @@ get_syscall_by_number (int syscall_number,
   init_sysinfo ();
 
   s->number = syscall_number;
-  s->name = xml_get_syscall_name (_sysinfo, syscall_number);
+  s->name = xml_get_syscall_name (sysinfo, syscall_number);
 }
 
 void
@@ -417,7 +407,7 @@ get_syscall_by_name (const char *syscall_name,
 {
   init_sysinfo ();
 
-  s->number = xml_get_syscall_number (_sysinfo, syscall_name);
+  s->number = xml_get_syscall_number (sysinfo, syscall_name);
   s->name = syscall_name;
 }
 
@@ -426,7 +416,7 @@ get_syscall_names (void)
 {
   init_sysinfo ();
 
-  return xml_list_of_syscalls (_sysinfo);
+  return xml_list_of_syscalls (sysinfo);
 }
 
 #endif /* ! HAVE_LIBEXPAT */