public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: binutils@sourceware.org
Subject: [PATCH 2/7] libctf: failure to open parent dicts that exist should be an error
Date: Fri, 26 Apr 2024 21:20:18 +0100	[thread overview]
Message-ID: <20240426202023.423064-3-nick.alcock@oracle.com> (raw)
In-Reply-To: <20240426202023.423064-1-nick.alcock@oracle.com>

CTF archive member opening (via ctf_arc_open_by_name, ctf_archive_iter, et
al) attempts to be helpful and auto-open and import any needed parent dict
in the same archive.  But if this fails, the error is not reported but
simply discarded, and you silently get back a dict with no parent, that
*you* suddenly have to remember to import.

This is not helpful behaviour: if the parent is corrupted or we run out of
memory or something, the caller is going to want to know!  Split it in two:
if the dict cites a parent that doesn't exist at all (a lot of historic
dicts name "PARENT" as their parent, even when they're not even children, or
perhaps the parent dict is stored separately and you plan to manually
associate it), we skip it as now, but if the import fails with an actual
error other than ECTF_ARNNAME, return the error and fail the open.

libctf/
	* ctf-archive.c (ctf_arc_import_parent):  Return failure if
        parent opening fails for reasons other thnn nonexistence.
	(ctf_dict_open_sections): Adjust.
---
 libctf/ctf-archive.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 3f12c0da85d..451d6c69735 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -44,7 +44,8 @@ static void *arc_mmap_file (int fd, size_t size);
 static int arc_mmap_writeout (int fd, void *header, size_t headersz,
 			      const char **errmsg);
 static int arc_mmap_unmap (void *header, size_t headersz, const char **errmsg);
-static void ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp);
+static int ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp,
+				  int *errp);
 
 /* Flag to indicate "symbol not present" in ctf_archive_internal.ctfi_symdicts
    and ctfi_symnamedicts.  Never initialized.  */
@@ -602,7 +603,11 @@ ctf_dict_open_sections (const ctf_archive_t *arc,
       if (ret)
 	{
 	  ret->ctf_archive = (ctf_archive_t *) arc;
-	  ctf_arc_import_parent (arc, ret);
+	  if (ctf_arc_import_parent (arc, ret, errp) < 0)
+	    {
+	      ctf_dict_close (ret);
+	      return NULL;
+	    }
 	}
       return ret;
     }
@@ -757,19 +762,26 @@ ctf_arc_open_by_name_sections (const ctf_archive_t *arc,
    already set, and a suitable archive member exists.  No error is raised if
    this is not possible: this is just a best-effort helper operation to give
    people useful dicts to start with.  */
-static void
-ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp)
+static int
+ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp, int *errp)
 {
   if ((fp->ctf_flags & LCTF_CHILD) && fp->ctf_parname && !fp->ctf_parent)
     {
+      int err;
       ctf_dict_t *parent = ctf_dict_open_cached ((ctf_archive_t *) arc,
-						 fp->ctf_parname, NULL);
+						 fp->ctf_parname, &err);
+      if (errp)
+	*errp = err;
+
       if (parent)
 	{
 	  ctf_import (fp, parent);
 	  ctf_dict_close (parent);
 	}
+      else if (err != ECTF_ARNNAME)
+	return -1;				/* errno is set for us.  */
     }
+  return 0;
 }
 
 /* Return the number of members in an archive.  */
-- 
2.44.0.273.ge0bd14271f


  parent reply	other threads:[~2024-04-26 20:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 20:20 [PATCH 0/7] libctf: leak-adjacent fixes Nick Alcock
2024-04-26 20:20 ` [PATCH 1/7] libctf: typos Nick Alcock
2024-04-26 20:20 ` Nick Alcock [this message]
2024-04-26 20:20 ` [PATCH 3/7] libctf: ctf_archive_iter: fix tiny leak Nick Alcock
2024-04-26 20:20 ` [PATCH 4/7] libctf: test: add lookup_link Nick Alcock
2024-04-26 20:20 ` [PATCH 5/7] libctf: test: add host Nick Alcock
2024-04-26 20:20 ` [PATCH 6/7] libctf: test: add wrapper Nick Alcock
2024-04-26 20:20 ` [PATCH 7/7] libctf: fix leak of entire dict when dict opening fails Nick Alcock

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240426202023.423064-3-nick.alcock@oracle.com \
    --to=nick.alcock@oracle.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).