Re: [PATCH 1/3] catch syscall -- try 5 -- Source code modifications

[Eli Zaretskii <eliz at gnu dot org>]

> From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= <sergiodj@linux.vnet.ibm.com>
> Date: Wed, 22 Apr 2009 21:33:03 -0300
> 
> Here goes the source-code modifications.

Thanks.  I have a few minor comments:

> 	(any_syscall_count, syscalls_counts,
> 	total_syscalls_count): New variables to keep track of requested
> 	syscall catchpoints.

The GNU Coding Standards specify the following formatting of log
entries with long lists that don't fit on a single line:

	(any_syscall_count, syscalls_counts)
	(total_syscalls_count): New variables to keep track of requested
	syscall catchpoints.

IOW, close the parens and re-open them on the next line.

> +/* We keep a count of the number of times the user has requested a
> +   particular syscall to be tracked, and pass this information to the
> +   target.  This lets capable targets implement filtering directly.  */

Could you please expand the last sentence in this comment?  What kind
of filtering we are talking here about?  I'm worried that someone who
might be willing to implement such filtering on a ``capable target''
won't understand how to go about that.

> +  add_catch_command ("syscall", _("\
> +Catch system calls.\n\
> +With an argument, catch only that syscall."),

I think we should tell in the doc string of this command something
about the format of the argument(s).

> +	  /* If we are catching this specific syscall number, then we
> +	     should update the target_status to reflect which event
> +	     has occurred.  But if this syscall is not to caught,
                                                   ^^^^^^^^^^^^^
"not to be caught"

> +	     This is needed so that GDB doesn't get confused when
> +	     the program is re-run'ed and no syscalls were caught
> +	     in the first run.  */

I think "re-ran" is better than "re-run'ed".  More importantly, I
don't understand why GDB would be confused by this sequence of events,
so perhaps you could expand the comment.

> +  /* Handle GNU/Linux's extended waitstatus for trace events.
> +     It is necessary to check if WSTOPSIG is signaling a that
                                                          ^
This "a" should be either removed or replaced by something else.