* Fwd: [PATCH v4 0/4] catch syscall group
[not found] <001a11c3b928756ec20515e95aba@google.com>
@ 2015-05-12 21:47 ` Doug Evans
2015-05-12 22:02 ` Sergio Durigan Junior
1 sibling, 0 replies; 6+ messages in thread
From: Doug Evans @ 2015-05-12 21:47 UTC (permalink / raw)
To: Gabriel Krisman Bertazi, gdb-patches; +Cc: Sergio Durigan Junior, Pedro Alves
Hi.
The message to the list bounced, but I'm not sure why (this is
text/plain not html).
Apologies for the resend.
---------- Forwarded message ----------
From: Doug Evans <dje@google.com>
Date: Tue, May 12, 2015 at 2:41 PM
Subject: Re: [PATCH v4 0/4] catch syscall group
To: Gabriel Krisman Bertazi <gabriel@krisman.be>
Cc: sergiodj@redhat.com, palves@redhat.com, gdb-patches@sourceware.org
Gabriel Krisman Bertazi writes:
> Thank you both for your review.
>
> This version has the following improvements:
>
> * Apply fixes suggested by Sergio in the testsuite.
> * Use xsltproc to generate the xml files.
>
> Regarding the last change, it allowed me to identify inconsistencies in
> groups for some architectures. The current design makes sure these
> inconsistencies are fixed by centralizing the group information in a
> single file.
>
> Also, this patch series *does not* include the generated files because
> they are too big and can get in the way of code review. Reviewers must
> generate those files by hand by entering the gdb/syscalls directory and
> running the makefile there. Build will fail if reviewer don't do this!
> Once we get this approved, I'll make sure to include the generated files
> in the commit before pushing. Hopefully this will make code review
> easier.
This sounds like something we should key off of --enable-maintainer-mode.
[we *could* use a different option if people are wedded to
--enable-maintainer-mode affecting only autogen files, but
that seems like overkill]
IIRC we don't do that for, e.g., gdbarch.sh -> gdbarch.[ch], but
that's a mistake IMO. Let's get it right here.
It will mean that configuring with --enable-maintainer-mode
will now require xsltproc, but that's the price of going down
this path, let's not hide it.
[maybe that's a good reason to use something other than
--enable-maintainer-mode, but
1) how often do people configure with --enable-maintainer-mode, and
2) maintainers are expected to know and accept these dependencies]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/4] catch syscall group
[not found] <001a11c3b928756ec20515e95aba@google.com>
2015-05-12 21:47 ` Fwd: [PATCH v4 0/4] catch syscall group Doug Evans
@ 2015-05-12 22:02 ` Sergio Durigan Junior
2015-05-12 22:44 ` Doug Evans
1 sibling, 1 reply; 6+ messages in thread
From: Sergio Durigan Junior @ 2015-05-12 22:02 UTC (permalink / raw)
To: Doug Evans; +Cc: Gabriel Krisman Bertazi, palves, gdb-patches
On Tuesday, May 12 2015, Doug Evans wrote:
> > Also, this patch series *does not* include the generated files because
> > they are too big and can get in the way of code review. Reviewers must
> > generate those files by hand by entering the gdb/syscalls directory and
> > running the makefile there. Build will fail if reviewer don't do this!
> > Once we get this approved, I'll make sure to include the generated files
> > in the commit before pushing. Hopefully this will make code review
> > easier.
>
> This sounds like something we should key off of --enable-maintainer-mode.
> [we *could* use a different option if people are wedded to
> --enable-maintainer-mode affecting only autogen files, but
> that seems like overkill]
What exactly are you refering to? Generating the XML files using
xsltproc when compiling GDB? I will assume this in the rest of the
message, but if that's not what you meant, then please disconsider.
> IIRC we don't do that for, e.g., gdbarch.sh -> gdbarch.[ch], but
> that's a mistake IMO. Let's get it right here.
Sorry, but what is the point of regenerating files that do not change
every time we compile GDB? I fail to see that.
> It will mean that configuring with --enable-maintainer-mode
> will now require xsltproc, but that's the price of going down
> this path, let's not hide it.
> [maybe that's a good reason to use something other than
> --enable-maintainer-mode, but
> 1) how often do people configure with --enable-maintainer-mode, and
> 2) maintainers are expected to know and accept these dependencies]
They will know and accept the dependencies if they really need to
regenerate the files, of course. Other than that, I still don't see any
practical reason to impose this on anyone who is compiling GDB, whether
this person is a GDB developer or not.
I don't necessarily oppose hooking the XML generation into the
--enable-maintainer-mode option, but I'm having the impression that we
are bloating this feature more and more, without much gain. Unless I'm
really blind to some benefit, in which case I apologize in advance.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/4] catch syscall group
2015-05-12 22:02 ` Sergio Durigan Junior
@ 2015-05-12 22:44 ` Doug Evans
2015-05-12 23:26 ` Sergio Durigan Junior
0 siblings, 1 reply; 6+ messages in thread
From: Doug Evans @ 2015-05-12 22:44 UTC (permalink / raw)
To: Sergio Durigan Junior; +Cc: Gabriel Krisman Bertazi, Pedro Alves, gdb-patches
On Tue, May 12, 2015 at 3:02 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Tuesday, May 12 2015, Doug Evans wrote:
>
>> > Also, this patch series *does not* include the generated files because
>> > they are too big and can get in the way of code review. Reviewers must
>> > generate those files by hand by entering the gdb/syscalls directory and
>> > running the makefile there. Build will fail if reviewer don't do this!
>> > Once we get this approved, I'll make sure to include the generated files
>> > in the commit before pushing. Hopefully this will make code review
>> > easier.
>>
>> This sounds like something we should key off of --enable-maintainer-mode.
>> [we *could* use a different option if people are wedded to
>> --enable-maintainer-mode affecting only autogen files, but
>> that seems like overkill]
>
> What exactly are you refering to? Generating the XML files using
> xsltproc when compiling GDB? I will assume this in the rest of the
> message, but if that's not what you meant, then please disconsider.
Ah. I figured people know what --enable-maintainer-mode is. :-)
--enable-maintainer-mode is a configure time option that turns
on some makefile dependency checking that is normally off.
It is used, for example, to automagically regenerate configure
when configure.ac changes, and only when make's
standard processing says they need regenerating: i.e.,
when the timestamp of the generated file is older than a timestamp
of one of its dependencies.
What happens if the user doesn't supply --enable-maintainer-mode
when configuring? [which is the norm]
Then the dependencies are turned off and no automagic regeneration
is done (which is what one would want for a default).
> I don't necessarily oppose hooking the XML generation into the
> --enable-maintainer-mode option, but I'm having the impression that we
> are bloating this feature more and more, without much gain. Unless I'm
> really blind to some benefit, in which case I apologize in advance.
What I'm asking for is trivial to do (there's already boilerplate
to cut-n-paste-n-tweak form), *and* it is s.o.p.
[If people want a different configure option than --enable-maintainer-mode
than it'll involve a bit more work, but it's still all s.o.p.]
I really don't think I'm asking for anything unusual or excessive.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/4] catch syscall group
2015-05-12 22:44 ` Doug Evans
@ 2015-05-12 23:26 ` Sergio Durigan Junior
0 siblings, 0 replies; 6+ messages in thread
From: Sergio Durigan Junior @ 2015-05-12 23:26 UTC (permalink / raw)
To: Doug Evans; +Cc: Gabriel Krisman Bertazi, Pedro Alves, gdb-patches
On Tuesday, May 12 2015, Doug Evans wrote:
>> What exactly are you refering to? Generating the XML files using
>> xsltproc when compiling GDB? I will assume this in the rest of the
>> message, but if that's not what you meant, then please disconsider.
>
> Ah. I figured people know what --enable-maintainer-mode is. :-)
I always knew GDB had it, but I confess I never tried it myself.
> --enable-maintainer-mode is a configure time option that turns
> on some makefile dependency checking that is normally off.
> It is used, for example, to automagically regenerate configure
> when configure.ac changes, and only when make's
> standard processing says they need regenerating: i.e.,
> when the timestamp of the generated file is older than a timestamp
> of one of its dependencies.
>
> What happens if the user doesn't supply --enable-maintainer-mode
> when configuring? [which is the norm]
> Then the dependencies are turned off and no automagic regeneration
> is done (which is what one would want for a default).
Right, thanks for explaining. I don't know if a lot of people use this
or not (it seems to me that they don't), but yeah, since we have it...
>> I don't necessarily oppose hooking the XML generation into the
>> --enable-maintainer-mode option, but I'm having the impression that we
>> are bloating this feature more and more, without much gain. Unless I'm
>> really blind to some benefit, in which case I apologize in advance.
>
> What I'm asking for is trivial to do (there's already boilerplate
> to cut-n-paste-n-tweak form), *and* it is s.o.p.
> [If people want a different configure option than --enable-maintainer-mode
> than it'll involve a bit more work, but it's still all s.o.p.]
>
> I really don't think I'm asking for anything unusual or excessive.
If what you're asking is trivial to do, then sure, I agree! I mean,
we've gone this far already, right? :-)
Thanks for the e-mail, and sorry if I sounded harsh.
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/4] catch syscall group
2015-05-11 0:28 ` [PATCH v4 0/4] catch " Gabriel Krisman Bertazi
@ 2015-05-13 10:47 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2015-05-13 10:47 UTC (permalink / raw)
To: Gabriel Krisman Bertazi, sergiodj; +Cc: gdb-patches, dje
On 05/11/2015 01:27 AM, Gabriel Krisman Bertazi wrote:
> Thank you both for your review.
>
> This version has the following improvements:
>
> * Apply fixes suggested by Sergio in the testsuite.
> * Use xsltproc to generate the xml files.
>
> Regarding the last change, it allowed me to identify inconsistencies in
> groups for some architectures.
It was a useful exercise then. ;-) Many thanks for following through
with this.
> The current design makes sure these
> inconsistencies are fixed by centralizing the group information in a
> single file.
>
> Also, this patch series *does not* include the generated files because
> they are too big and can get in the way of code review. Reviewers must
> generate those files by hand by entering the gdb/syscalls directory and
> running the makefile there. Build will fail if reviewer don't do this!
> Once we get this approved, I'll make sure to include the generated files
> in the commit before pushing. Hopefully this will make code review
> easier.
Do you have a public branch somewhere with that already done?
That'd make it easier for people to check the end result. You could push
it to sourceware.org git nowadays, under "users/$yourname/$branch". That
would be the easiest.
> Also, I generated this format-patch with --find-renames. If it gets in
> the way of git am/git apply, please let me know and I'll resend it.
>
> As usual, I'm not resending the documentation patch again because it was
> already approved by Eli.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 0/4] catch syscall group
2015-05-10 19:01 [PATCH v3 00/17] Catch " Sergio Durigan Junior
@ 2015-05-11 0:28 ` Gabriel Krisman Bertazi
2015-05-13 10:47 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2015-05-11 0:28 UTC (permalink / raw)
To: sergiodj; +Cc: palves, gdb-patches, dje, Gabriel Krisman Bertazi
Thank you both for your review.
This version has the following improvements:
* Apply fixes suggested by Sergio in the testsuite.
* Use xsltproc to generate the xml files.
Regarding the last change, it allowed me to identify inconsistencies in
groups for some architectures. The current design makes sure these
inconsistencies are fixed by centralizing the group information in a
single file.
Also, this patch series *does not* include the generated files because
they are too big and can get in the way of code review. Reviewers must
generate those files by hand by entering the gdb/syscalls directory and
running the makefile there. Build will fail if reviewer don't do this!
Once we get this approved, I'll make sure to include the generated files
in the commit before pushing. Hopefully this will make code review
easier.
Also, I generated this format-patch with --find-renames. If it gets in
the way of git am/git apply, please let me know and I'll resend it.
As usual, I'm not resending the documentation patch again because it was
already approved by Eli.
Thanks!
Gabriel Krisman Bertazi (4):
Implemement support for groups of syscalls in the xml-syscall
interface.
Add support to catch groups of syscalls.
Add tests for catching groups of syscalls on supported
architectures.
Include group information in xml syscall files.
gdb/break-catch-syscall.c | 94 +++++++-
gdb/syscalls/Makefile | 42 ++++
.../{aarch64-linux.xml => aarch64-linux.xml.in} | 0
.../{amd64-linux.xml => amd64-linux.xml.in} | 0
gdb/syscalls/apply-defaults.xsl | 27 +++
gdb/syscalls/{arm-linux.xml => arm-linux.xml.in} | 0
gdb/syscalls/{bfin-linux.xml => bfin-linux.xml.in} | 0
gdb/syscalls/gdb-syscalls.dtd | 3 +-
gdb/syscalls/{i386-linux.xml => i386-linux.xml.in} | 0
gdb/syscalls/linux-defaults.xml.in | 243 +++++++++++++++++++++
.../{mips-n32-linux.xml => mips-n32-linux.xml.in} | 0
.../{mips-n64-linux.xml => mips-n64-linux.xml.in} | 0
.../{mips-o32-linux.xml => mips-o32-linux.xml.in} | 0
gdb/syscalls/{ppc-linux.xml => ppc-linux.xml.in} | 0
.../{ppc64-linux.xml => ppc64-linux.xml.in} | 0
gdb/syscalls/{s390-linux.xml => s390-linux.xml.in} | 0
.../{s390x-linux.xml => s390x-linux.xml.in} | 0
.../{sparc-linux.xml => sparc-linux.xml.in} | 0
.../{sparc64-linux.xml => sparc64-linux.xml.in} | 0
gdb/testsuite/gdb.base/catch-syscall.exp | 39 ++++
gdb/xml-syscall.c | 231 +++++++++++++++++++-
gdb/xml-syscall.h | 16 ++
22 files changed, 683 insertions(+), 12 deletions(-)
create mode 100644 gdb/syscalls/Makefile
rename gdb/syscalls/{aarch64-linux.xml => aarch64-linux.xml.in} (100%)
rename gdb/syscalls/{amd64-linux.xml => amd64-linux.xml.in} (100%)
create mode 100644 gdb/syscalls/apply-defaults.xsl
rename gdb/syscalls/{arm-linux.xml => arm-linux.xml.in} (100%)
rename gdb/syscalls/{bfin-linux.xml => bfin-linux.xml.in} (100%)
rename gdb/syscalls/{i386-linux.xml => i386-linux.xml.in} (100%)
create mode 100644 gdb/syscalls/linux-defaults.xml.in
rename gdb/syscalls/{mips-n32-linux.xml => mips-n32-linux.xml.in} (100%)
rename gdb/syscalls/{mips-n64-linux.xml => mips-n64-linux.xml.in} (100%)
rename gdb/syscalls/{mips-o32-linux.xml => mips-o32-linux.xml.in} (100%)
rename gdb/syscalls/{ppc-linux.xml => ppc-linux.xml.in} (100%)
rename gdb/syscalls/{ppc64-linux.xml => ppc64-linux.xml.in} (100%)
rename gdb/syscalls/{s390-linux.xml => s390-linux.xml.in} (100%)
rename gdb/syscalls/{s390x-linux.xml => s390x-linux.xml.in} (100%)
rename gdb/syscalls/{sparc-linux.xml => sparc-linux.xml.in} (100%)
rename gdb/syscalls/{sparc64-linux.xml => sparc64-linux.xml.in} (100%)
--
2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-13 10:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <001a11c3b928756ec20515e95aba@google.com>
2015-05-12 21:47 ` Fwd: [PATCH v4 0/4] catch syscall group Doug Evans
2015-05-12 22:02 ` Sergio Durigan Junior
2015-05-12 22:44 ` Doug Evans
2015-05-12 23:26 ` Sergio Durigan Junior
2015-05-10 19:01 [PATCH v3 00/17] Catch " Sergio Durigan Junior
2015-05-11 0:28 ` [PATCH v4 0/4] catch " Gabriel Krisman Bertazi
2015-05-13 10:47 ` Pedro Alves
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).