Re: problems with v3
I don't know this area well, but considering that ser-unix.c is just chock full of tty-related goo, I think it is probably important for something. My impression is that this API is not just used for target communication but also for manipulating gdb's own terminal. Ah, I see. I've appended the patch I came up with. I have not tried it at all, but it should all be pretty obvious. Yeah, that seems like it should be reasonable, i.e. more or less like what I'd figured it would do if it didn't have all these different backends. However, I think the test you probably want is !S_ISCHR. I believe that /proc/ugdb as is stats as S_ISREG, not S_ISFIFO. Actually, better yet, just make it !isatty (fd). Another pseudodevice that also behaves more like a socket than like a tty might be S_ISCHR, but only a tty isatty. Thanks, Roland
Re: problems with v3
Roland However, I think the test you probably want is !S_ISCHR. Thanks. I wasn't sure. Roland Actually, better yet, just make it !isatty (fd). Ok. I made a new branch, 'archer-ugdb'. This patch is there. (Actually 2 patches due to me not reading carefully enough the first time -- but whatever.) Oleg, use this branch and give the feature a try. We can push whatever fixes or hacks you need to this branch. I can also merge from gdb's master as needed. Tom
Re: problems with v3
On 08/13, Tom Tromey wrote: Roland However, I think the test you probably want is !S_ISCHR. Thanks. I wasn't sure. Roland Actually, better yet, just make it !isatty (fd). Ok. Good. IIUC, this also allows to remove the ugly return 0 to pretend TCGETS/TCSETS/TCFLSH works from -ioctl(). I made a new branch, 'archer-ugdb'. I assume, you mean git://sourceware.org/git/gdb.git ? This patch is there. (Actually 2 patches due to me not reading carefully enough the first time -- but whatever.) Oleg, use this branch and give the feature a try. Yes, once I resolve the current problems I'll test it. Thanks! Oleg.
Re: problems with v3 (Was: gdbstub initial code, v3)
I seem to understand the problem(s). I am a bit surprized. Basically I have the questions about almost every utrace_ function. I'll try to recheck and summarize my concerns tomorrow with a fresh head, I hope I missed something. Ok. That stuff about pure kernel implementation issues doesn't need to go to the archer list and Tom, only to utrace-devel. Thanks, Roland
[PATCH 1/4] utrace_set_events: make the death/reap checks readable
No changes in the compiled code. Just make the code parsable by humans.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
--- kstub/kernel/utrace.c~1_use_cleanup_exit_state_checks 2010-08-14
04:28:58.0 +0200
+++ kstub/kernel/utrace.c 2010-08-14 04:30:28.0 +0200
@@ -539,13 +539,15 @@ int utrace_set_events(struct task_struct
old_utrace_flags = target-utrace_flags;
old_flags = engine-flags ~ENGINE_STOP;
- if (target-exit_state
- (((events ~old_flags) _UTRACE_DEATH_EVENTS) ||
-(utrace-death
- ((old_flags ~events) _UTRACE_DEATH_EVENTS)) ||
-(utrace-reap ((old_flags ~events) UTRACE_EVENT(REAP) {
- spin_unlock(utrace-lock);
- return -EALREADY;
+ if (target-exit_state) {
+ unsigned long cleared = (old_flags ~events);
+
+ if (((events ~old_flags) _UTRACE_DEATH_EVENTS) ||
+ (utrace-death (cleared _UTRACE_DEATH_EVENTS)) ||
+ (utrace-reap (cleared UTRACE_EVENT(REAP {
+ spin_unlock(utrace-lock);
+ return -EALREADY;
+ }
}
/*
[PATCH 3/4] utrace_set_events: fix UTRACE_EVENT(REAP) case
I am not sure this fix is really needed, up to you.
But please note that this code
if ((utrace-death (cleared _UTRACE_DEATH_EVENTS)) ||
(utrace-reap (cleared UTRACE_EVENT(REAP {
spin_unlock(utrace-lock);
return -EALREADY;
}
is not exactly right, it has 2 minor problems:
- It is possible that both -death and -reap are true. In this
case it is OK to clear UTRACE_EVENT(REAP), but set_events fails.
- OTOH, if -reap is true, set_events disallows clear but allows
set, the latter case should be forbidden too.
If utrace_set_events(UTRACE_EVENT(REAP)) succeeds, the caller
has all rights to expect -report_reap() will be caller later.
If you take this patch, then please consider the next one.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
kernel/utrace.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
--- kstub/kernel/utrace.c~3_minor_death_reap_fix2010-08-14
04:30:28.0 +0200
+++ kstub/kernel/utrace.c 2010-08-14 04:30:29.0 +0200
@@ -541,12 +541,16 @@ int utrace_set_events(struct task_struct
/* If -death or -reap is true we must see exit_state != 0. */
if (target-exit_state) {
- unsigned long cleared = (old_flags ~events);
-
- if ((utrace-death (cleared _UTRACE_DEATH_EVENTS)) ||
- (utrace-reap (cleared UTRACE_EVENT(REAP {
- spin_unlock(utrace-lock);
- return -EALREADY;
+ if (utrace-death) {
+ if ((old_flags ~events) _UTRACE_DEATH_EVENTS) {
+ spin_unlock(utrace-lock);
+ return -EALREADY;
+ }
+ } else if (utrace-reap) {
+ if ((old_flags ^ events) UTRACE_EVENT(REAP)) {
+ spin_unlock(utrace-lock);
+ return -EALREADY;
+ }
}
}
