[binutils-gdb] libctf: archive, open: when opening, always set errp to something

[Nick Alcock]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=14303d6295e1bfd8a4e1b447057b78ce583be75d

commit 14303d6295e1bfd8a4e1b447057b78ce583be75d
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Mon May 12 12:31:00 2025 +0100

    libctf: archive, open: when opening, always set errp to something
    
    ctf_arc_import_parent, called by the cached-opening machinery used by
    ctf_archive_next and archive-wide lookup functions like
    ctf_arc_lookup_symbol, has an err-pointer parameter like all other opening
    functions.  Unfortunately it unconditionally initializes it whenever
    provided, even if there was no error, which can lead to its being
    initialized to an uninitialized value.  This is not technically an
    API-contract violation, since we don't define what happens to the error
    value except when an error happens, but it is still unpleasant.
    
    Initialize it only when there is an actual error, so we never initialize it
    to an uninitialized value.
    
    While we're at it, improve all the opening pathways: on success, set errp to
    0, rather than leaving it what it was, reducing the likelihood of
    uninitialized error param returns in callers too.  (This is inconsistent
    with the treatment of ctf_errno(), but the err value being a parameter
    passed in from outside makes the divergence acceptable: in open functions,
    you're never going to be overwriting some old error value someone might want
    to keep around across multiple calls, some of which are successful and some
    of which are not.)
    
    Soup up existing tests to verify all this.
    
    Thanks to Bruce McCulloch for the original patch, and Stephen Brennan for
    the report.
    
    libctf/
            PR libctf/32903
            * ctf-archive.c (ctf_arc_open_internal): Zero errp on success.
            (ctf_dict_open_sections): Zero errp at the start.
            (ctf_arc_import_parent): Intialize err.
            * ctf-open.c (ctf_bufopen): Zero errp at the start.
            * testsuite/libctf-lookup/add-to-opened.c: Make sure one-element
            archive opens update errp.
            * testsuite/libctf-writable/ctf-compressed.c: Make sure real archive
            opens update errp.

Diff:
---
 libctf/ctf-archive.c                              |  9 ++++++++-
 libctf/ctf-open.c                                 |  2 ++
 libctf/testsuite/libctf-lookup/add-to-opened.c    | 15 ++++++++++++++-
 libctf/testsuite/libctf-writable/ctf-compressed.c |  4 ++++
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index dc01e2e0b08..6c4595fcc84 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -485,6 +485,10 @@ ctf_arc_open_internal (const char *filename, int *errp)
      is private.)  */
   arc->ctfa_magic = s.st_size;
   close (fd);
+
+  if (errp)
+    *errp = 0;
+
   return arc;
 
 err_unmap:
@@ -588,6 +592,9 @@ ctf_dict_open_sections (const ctf_archive_t *arc,
 			const char *name,
 			int *errp)
 {
+  if (errp)
+    *errp = 0;
+
   if (arc->ctfi_is_archive)
     {
       ctf_dict_t *ret;
@@ -761,7 +768,7 @@ 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;
+      int err = 0;
       ctf_dict_t *parent = ctf_dict_open_cached ((ctf_archive_t *) arc,
 						 fp->ctf_parname, &err);
       if (errp)
diff --git a/libctf/ctf-open.c b/libctf/ctf-open.c
index 449728fc3f4..8c086674836 100644
--- a/libctf/ctf-open.c
+++ b/libctf/ctf-open.c
@@ -1426,6 +1426,8 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
 
   libctf_init_debug();
 
+  ctf_set_open_errno (errp, 0);
+
   if ((ctfsect == NULL) || ((symsect != NULL) && (strsect == NULL)))
     return (ctf_set_open_errno (errp, EINVAL));
 
diff --git a/libctf/testsuite/libctf-lookup/add-to-opened.c b/libctf/testsuite/libctf-lookup/add-to-opened.c
index 96629afe1aa..700257e8a0e 100644
--- a/libctf/testsuite/libctf-lookup/add-to-opened.c
+++ b/libctf/testsuite/libctf-lookup/add-to-opened.c
@@ -15,7 +15,7 @@ main (int argc, char *argv[])
   ctf_encoding_t en = { CTF_INT_SIGNED, 0, sizeof (int) };
   unsigned char *ctf_written;
   size_t size;
-  int err;
+  int err = 666;
 
   if (argc != 2)
     {
@@ -25,9 +25,18 @@ main (int argc, char *argv[])
 
   if ((ctf = ctf_open (argv[1], NULL, &err)) == NULL)
     goto open_err;
+
+  /* The error int should be reset on success as well as on error.  */
+  if (err != 0)
+    goto err_err;
+
+  err = 666;
   if ((fp = ctf_dict_open (ctf, NULL, &err)) == NULL)
     goto open_err;
 
+  if (err != 0)
+    goto err_err;
+
   /* Check that various modifications to already-written types
      are prohibited.  */
 
@@ -145,4 +154,8 @@ main (int argc, char *argv[])
  open_err:
   fprintf (stderr, "%s: cannot open: %s\n", argv[0], ctf_errmsg (err));
   return 1;
+
+ err_err:
+  fprintf (stderr, "%s: open error not set to success on success\n", argv[0]);
+  return 1;
 }    
diff --git a/libctf/testsuite/libctf-writable/ctf-compressed.c b/libctf/testsuite/libctf-writable/ctf-compressed.c
index 4769cdb97f2..646b6035d14 100644
--- a/libctf/testsuite/libctf-writable/ctf-compressed.c
+++ b/libctf/testsuite/libctf-writable/ctf-compressed.c
@@ -119,10 +119,14 @@ main (int argc, char *argv[])
 
   /* Dump the header of each archive member, and search for CTF_F_COMPRESS in
      the resulting dump.  */
+  err = 666;
   while ((dump_fp = ctf_archive_next (final_arc, &i, NULL, 0, &err)) != NULL)
     {
       char *dumpstr;
 
+      if (err != 0)
+	fprintf (stderr, "err not set to success on success\n");
+
       while ((dumpstr = ctf_dump (dump_fp, &dump_state, CTF_SECT_HEADER,
 				  NULL, NULL)) != NULL)
 	{