RFA: patch to infptrace.c

RFA: patch to infptrace.c

[J. Johnston]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

The attached patch changes the ptrace detach call to not perform error handling
when an error occurs in detach.  Without this patch, an error in detach stops gdb
from exiting.  The error keeps returning to the command line over and over again.

I thought I had fixed this earlier by adding a check for errno != ESRCH, but it
appears there are other scenarios that may occur.

Ok to commit?

-- Jeff J.

2003-05-21  Jeff Johnston  <jjohnstn@redhat.com>

	* infptrace.c (detach): Call print_sys_errmsg when an error occurs
	instead of perror_with_name.

[-- Attachment #2: infptrace.patch --]
[-- Type: text/plain, Size: 559 bytes --]

Index: infptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/infptrace.c,v
retrieving revision 1.25
diff -u -p -r1.25 infptrace.c
--- infptrace.c	8 May 2003 20:52:47 -0000	1.25
+++ infptrace.c	21 May 2003 21:39:56 -0000
@@ -302,7 +302,7 @@ detach (int signal)
   ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
           signal);
   if (errno && errno != ESRCH)
-    perror_with_name ("ptrace");
+    print_sys_errmsg ("ptrace", errno);
   attach_flag = 0;
 }
 #endif /* ATTACH_DETACH */

Re: RFA: patch to infptrace.c

[Kevin Buettner]

On May 21,  6:31pm, J. Johnston wrote:

> The attached patch changes the ptrace detach call to not perform error handling
> when an error occurs in detach.  Without this patch, an error in detach stops gdb
> from exiting.  The error keeps returning to the command line over and over again.
> 
> I thought I had fixed this earlier by adding a check for errno != ESRCH, but it
> appears there are other scenarios that may occur.

Which other errno values are tripping it up?

Assuming that it's a good idea to emit only a warning, do we still need
the errno != ESRCH check?

> Ok to commit?
> 
> -- Jeff J.
> 
> 2003-05-21  Jeff Johnston  <jjohnstn@redhat.com>
> 
> 	* infptrace.c (detach): Call print_sys_errmsg when an error occurs
> 	instead of perror_with_name.
> 
> [ text/plain ] :
> 
> Index: infptrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infptrace.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 infptrace.c
> --- infptrace.c	8 May 2003 20:52:47 -0000	1.25
> +++ infptrace.c	21 May 2003 21:39:56 -0000
> @@ -302,7 +302,7 @@ detach (int signal)
>    ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
>            signal);
>    if (errno && errno != ESRCH)
> -    perror_with_name ("ptrace");
> +    print_sys_errmsg ("ptrace", errno);
>    attach_flag = 0;
>  }
>  #endif /* ATTACH_DETACH */

Re: RFA: patch to infptrace.c

[J. Johnston]

Kevin Buettner wrote:
> On May 21,  6:31pm, J. Johnston wrote:
> 
> 
>>The attached patch changes the ptrace detach call to not perform error handling
>>when an error occurs in detach.  Without this patch, an error in detach stops gdb
>>from exiting.  The error keeps returning to the command line over and over again.
>>
>>I thought I had fixed this earlier by adding a check for errno != ESRCH, but it
>>appears there are other scenarios that may occur.
> 
> 
> Which other errno values are tripping it up?
> 

A bugzilla bug has been opened on a Red Hat kernel with patches that was
causing an EPERM to show up.

> Assuming that it's a good idea to emit only a warning, do we still need
> the errno != ESRCH check?
>

It isn't "needed".  It was meant to not issue any complaint if the process had
been killed externally.  I can remove the check if it is preferred.

> 
>>Ok to commit?
>>
>>-- Jeff J.
>>
>>2003-05-21  Jeff Johnston  <jjohnstn@redhat.com>
>>
>>	* infptrace.c (detach): Call print_sys_errmsg when an error occurs
>>	instead of perror_with_name.
>>
>>[ text/plain ] :
>>
>>Index: infptrace.c
>>===================================================================
>>RCS file: /cvs/src/src/gdb/infptrace.c,v
>>retrieving revision 1.25
>>diff -u -p -r1.25 infptrace.c
>>--- infptrace.c	8 May 2003 20:52:47 -0000	1.25
>>+++ infptrace.c	21 May 2003 21:39:56 -0000
>>@@ -302,7 +302,7 @@ detach (int signal)
>>   ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
>>           signal);
>>   if (errno && errno != ESRCH)
>>-    perror_with_name ("ptrace");
>>+    print_sys_errmsg ("ptrace", errno);
>>   attach_flag = 0;
>> }
>> #endif /* ATTACH_DETACH */
> 
> 

Re: RFA: patch to infptrace.c

[Kevin Buettner]

On May 21,  7:11pm, J. Johnston wrote:

> Kevin Buettner wrote:
> > On May 21,  6:31pm, J. Johnston wrote:
> > 
> > 
> >>The attached patch changes the ptrace detach call to not perform error handling
> >>when an error occurs in detach.  Without this patch, an error in detach stops gdb
> >>from exiting.  The error keeps returning to the command line over and over again.
> >>
> >>I thought I had fixed this earlier by adding a check for errno != ESRCH, but it
> >>appears there are other scenarios that may occur.
> > 
> > 
> > Which other errno values are tripping it up?
> > 
> 
> A bugzilla bug has been opened on a Red Hat kernel with patches that was
> causing an EPERM to show up.
> 
> > Assuming that it's a good idea to emit only a warning, do we still need
> > the errno != ESRCH check?
> 
> It isn't "needed".  It was meant to not issue any complaint if the process had
> been killed externally.  I can remove the check if it is preferred.

If the process had been killed externally and the gdb user was unaware
of it, then I think this information would be quite useful.  It's
certainly better (less surprising) to print a message than to detach
expecting the process to continue on it's own only to find it
mysteriously gone.  It won't really help to tell the user why it's
gone, but the user will at least know that it happened before the
detach or possibly as a result of something having gone wrong with the
detach.

Anyway, I'm all for your patch going in so long as the ESRCH check is
removed.  It doesn't make any sense to put the GDB user in limbo as
a result of being unable to detach successfully.  When I've encountered
this problem in the past, there didn't seem to be anything that could
be done (as a user) to correct the problem so that the detach would
succeed.

Kevin

> 
> > 
> >>Ok to commit?
> >>
> >>-- Jeff J.
> >>
> >>2003-05-21  Jeff Johnston  <jjohnstn@redhat.com>
> >>
> >>	* infptrace.c (detach): Call print_sys_errmsg when an error occurs
> >>	instead of perror_with_name.
> >>
> >>[ text/plain ] :
> >>
> >>Index: infptrace.c
> >>===================================================================
> >>RCS file: /cvs/src/src/gdb/infptrace.c,v
> >>retrieving revision 1.25
> >>diff -u -p -r1.25 infptrace.c
> >>--- infptrace.c	8 May 2003 20:52:47 -0000	1.25
> >>+++ infptrace.c	21 May 2003 21:39:56 -0000
> >>@@ -302,7 +302,7 @@ detach (int signal)
> >>   ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
> >>           signal);
> >>   if (errno && errno != ESRCH)
> >>-    perror_with_name ("ptrace");
> >>+    print_sys_errmsg ("ptrace", errno);
> >>   attach_flag = 0;
> >> }
> >> #endif /* ATTACH_DETACH */
> > 
> > 
> 
>-- End of excerpt from J. Johnston

Re: RFA: patch to infptrace.c

[J. Johnston]

[-- Attachment #1: Type: text/plain, Size: 3083 bytes --]

The attached patch has been checked in.  Thanks.

-- Jeff J.

2003-05-22  Jeff Johnston  <jjohnstn@redhat.com>

         * infptrace.c (detach): Call print_sys_errmsg rather than
         perror_with_name to issue warning message when errno is non-zero
         after calling ptrace detach.



Kevin Buettner wrote:
> On May 21,  7:11pm, J. Johnston wrote:
> 
> 
>>Kevin Buettner wrote:
>>
>>>On May 21,  6:31pm, J. Johnston wrote:
>>>
>>>
>>>
>>>>The attached patch changes the ptrace detach call to not perform error handling
>>>>when an error occurs in detach.  Without this patch, an error in detach stops gdb
>>>
>>>>from exiting.  The error keeps returning to the command line over and over again.
>>>
>>>>I thought I had fixed this earlier by adding a check for errno != ESRCH, but it
>>>>appears there are other scenarios that may occur.
>>>
>>>
>>>Which other errno values are tripping it up?
>>>
>>
>>A bugzilla bug has been opened on a Red Hat kernel with patches that was
>>causing an EPERM to show up.
>>
>>
>>>Assuming that it's a good idea to emit only a warning, do we still need
>>>the errno != ESRCH check?
>>
>>It isn't "needed".  It was meant to not issue any complaint if the process had
>>been killed externally.  I can remove the check if it is preferred.
> 
> 
> If the process had been killed externally and the gdb user was unaware
> of it, then I think this information would be quite useful.  It's
> certainly better (less surprising) to print a message than to detach
> expecting the process to continue on it's own only to find it
> mysteriously gone.  It won't really help to tell the user why it's
> gone, but the user will at least know that it happened before the
> detach or possibly as a result of something having gone wrong with the
> detach.
> 
> Anyway, I'm all for your patch going in so long as the ESRCH check is
> removed.  It doesn't make any sense to put the GDB user in limbo as
> a result of being unable to detach successfully.  When I've encountered
> this problem in the past, there didn't seem to be anything that could
> be done (as a user) to correct the problem so that the detach would
> succeed.
> 
> Kevin
> 
> 
>>>>Ok to commit?
>>>>
>>>>-- Jeff J.
>>>>
>>>>2003-05-21  Jeff Johnston  <jjohnstn@redhat.com>
>>>>
>>>>	* infptrace.c (detach): Call print_sys_errmsg when an error occurs
>>>>	instead of perror_with_name.
>>>>
>>>>[ text/plain ] :
>>>>
>>>>Index: infptrace.c
>>>>===================================================================
>>>>RCS file: /cvs/src/src/gdb/infptrace.c,v
>>>>retrieving revision 1.25
>>>>diff -u -p -r1.25 infptrace.c
>>>>--- infptrace.c	8 May 2003 20:52:47 -0000	1.25
>>>>+++ infptrace.c	21 May 2003 21:39:56 -0000
>>>>@@ -302,7 +302,7 @@ detach (int signal)
>>>>  ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
>>>>          signal);
>>>>  if (errno && errno != ESRCH)
>>>>-    perror_with_name ("ptrace");
>>>>+    print_sys_errmsg ("ptrace", errno);
>>>>  attach_flag = 0;
>>>>}
>>>>#endif /* ATTACH_DETACH */
>>>
>>>
>>-- End of excerpt from J. Johnston
> 
> 
> 
> 


[-- Attachment #2: infptrace.patch --]
[-- Type: text/plain, Size: 587 bytes --]

Index: infptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/infptrace.c,v
retrieving revision 1.25
diff -u -p -r1.25 infptrace.c
--- infptrace.c	8 May 2003 20:52:47 -0000	1.25
+++ infptrace.c	22 May 2003 15:45:51 -0000
@@ -301,8 +301,8 @@ detach (int signal)
   errno = 0;
   ptrace (PT_DETACH, PIDGET (inferior_ptid), (PTRACE_ARG3_TYPE) 1,
           signal);
-  if (errno && errno != ESRCH)
-    perror_with_name ("ptrace");
+  if (errno)
+    print_sys_errmsg ("ptrace", errno);
   attach_flag = 0;
 }
 #endif /* ATTACH_DETACH */