Re: [PATCH] libdwfl: dwfl_standard_argp should not fail when not able to attach Dwfl.

Re: [PATCH] libdwfl: dwfl_standard_argp should not fail when not able to attach Dwfl.

[Mark Wielaard]

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

On Tue, 2014-06-10 at 15:21 +0200, Mark Wielaard wrote:
> As pointed out in https://bugzilla.redhat.com/show_bug.cgi?id=1107654
> commit 191080 introduced a thinko that caused dwfl_standard_argp
> to fail if the Dwfl couldn't be attached. Instead of generating a warning
> as the comment intended, the failure would be fatal. But even warning
> about dwfl_core_file_attach () or dwfl_linux_proc_attach () failing
> would be a mistake. The caller/user might not be interested in such
> a non-fatal issue. So just ignore if the call failed for whatever reason.
> If the caller is interested in warning up front about this issue, then
> dwfl_pid () should be called to check the Dwfl is attached. Things should
> work just fine for anything that doesn't call any of the dwfl_state related
> functions.
> 
> Signed-off-by: Mark Wielaard <mjw@redhat.com>
> ---
>  libdwfl/ChangeLog  |    5 +++++
>  libdwfl/argp-std.c |   13 +++++--------
>  2 files changed, 10 insertions(+), 8 deletions(-)

I pushed this patch and the followup "libdwfl: Record dwfl_attach_state
error and return it on failure." patch to master now.

Cheers,

Mark

[PATCH] libdwfl: dwfl_standard_argp should not fail when not able to attach Dwfl.

[Mark Wielaard]

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

As pointed out in https://bugzilla.redhat.com/show_bug.cgi?id=1107654
commit 191080 introduced a thinko that caused dwfl_standard_argp
to fail if the Dwfl couldn't be attached. Instead of generating a warning
as the comment intended, the failure would be fatal. But even warning
about dwfl_core_file_attach () or dwfl_linux_proc_attach () failing
would be a mistake. The caller/user might not be interested in such
a non-fatal issue. So just ignore if the call failed for whatever reason.
If the caller is interested in warning up front about this issue, then
dwfl_pid () should be called to check the Dwfl is attached. Things should
work just fine for anything that doesn't call any of the dwfl_state related
functions.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog  |    5 +++++
 libdwfl/argp-std.c |   13 +++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 2fd2a1a..c13e01f 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2014-06-10  Mark Wielaard  <mjw@redhat.com>
+
+	* argp-std.c (parse_opt): Ignore errors from dwfl_core_file_attach
+	or dwfl_linux_proc_attach.
+
 2014-05-15  Mark Wielaard  <mjw@redhat.com>
 
 	* linux-proc-maps.c (grovel_auxv): Close fd on error.
diff --git a/libdwfl/argp-std.c b/libdwfl/argp-std.c
index 8d2bc6a..42b7e78 100644
--- a/libdwfl/argp-std.c
+++ b/libdwfl/argp-std.c
@@ -171,10 +171,9 @@ parse_opt (int key, char *arg, struct argp_state *state)
 	    if (result != 0)
 	      return fail (dwfl, result, arg);
 
-	    result = INTUSE(dwfl_linux_proc_attach) (dwfl, atoi (arg), false);
-	    if (result != 0)
-	      /* Non-fatal to not be able to attach to process.  */
-	      failure (dwfl, result, _("cannot attach to process"));
+	    /* Non-fatal to not be able to attach to process, ignore error.  */
+	    INTUSE(dwfl_linux_proc_attach) (dwfl, atoi (arg), false);
+
 	    opt->dwfl = dwfl;
 	  }
 	else
@@ -301,10 +300,8 @@ parse_opt (int key, char *arg, struct argp_state *state)
 		return fail (dwfl, result, opt->core);
 	      }
 
-	    result = INTUSE(dwfl_core_file_attach) (dwfl, core);
-	    if (result < 0)
-	      /* Non-fatal to not be able to attach to core.  */
-	      failure (dwfl, result, _("cannot attach to core"));
+	    /* Non-fatal to not be able to attach to core, ignore error.  */
+	    INTUSE(dwfl_core_file_attach) (dwfl, core);
 
 	    /* From now we leak FD and CORE.  */
 
-- 
1.7.1