[02/12] libctf, include, binutils, gdb: rename CTF-opening functions

Message ID 20201025141413.363381-3-nick.alcock@oracle.com
State New
Headers show
Series
  • CTF symbol functionality
Related show

Commit Message

H.J. Lu via Binutils Oct. 25, 2020, 2:14 p.m.
The functions that return ctf_dict_t's given a ctf_archive_t and a name
are very clumsily named.  It sounds like they return *archives*, not
dictionaries, and the names are very long and clunky.  Why do we
have a ctf_arc_open_by_name when it opens a dictionary, not an archive,
and when there is no way to open a dictionary in any other way?  The
answer is purely internal: the function is located in ctf-archive.c,
and everything in there was called ctf_arc_*, and there is another
way to open a dict (by offset in the archive), that is internal to
ctf-archive.c and that nothing else can call.

This is clearly bad naming. The internal organization of the source tree
should not dictate public API names!

So rename things (keeping the old, bad names for compatibility), and
adjust all users.  You now open a dict using ctf_dict_open, and
open it giving ELF sections via ctf_dict_open_sections.

binutils/ChangeLog
2020-10-23  Nick Alcock  <nick.alcock@oracle.com>

	* objdump.c (dump_ctf): Use ctf_dict_open, not
	ctf_arc_open_by_name.
	* readelf.c (dump_section_as_ctf): Likewise.

gdb/ChangeLog
2020-10-23  Nick Alcock  <nick.alcock@oracle.com>

	* ctfread.c (elfctf_build_psymtabs): Use ctf_dict_open, not
	ctf_arc_open_by_name.

include/ChangeLog
2020-10-23  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-api.h (ctf_arc_open_by_name): Rename to...
	(ctf_dict_open): ... this, keeping compatibility function.
	(ctf_arc_open_by_name_sections): Rename to...
	(ctf_dict_open_sections): ... this, keeping compatibility function.

libctf/ChangeLog
2020-10-23  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-archive.c (ctf_arc_open_by_offset): Rename to...
	(ctf_dict_open_by_offset): ... this.  Adjust callers.
	(ctf_arc_open_by_name_internal): Rename to...
	(ctf_dict_open_internal): ... this.  Adjust callers.
	(ctf_arc_open_by_name_sections): Rename to...
	(ctf_dict_open_sections): ... this, keeping compatibility function.
	(ctf_arc_open_by_name): Rename to...
	(ctf_dict_open): ... this, keeping compatibility function.
	* libctf.ver: New functions added.
	* ctf-link.c (ctf_link_one_input_archive): Adjusted accordingly.
	(ctf_link_deduplicating_open_inputs): Likewise.
---
 binutils/objdump.c   |  2 +-
 binutils/readelf.c   |  2 +-
 gdb/ctfread.c        |  4 +--
 include/ctf-api.h    | 18 +++++++----
 libctf/ctf-archive.c | 75 +++++++++++++++++++++++++++-----------------
 libctf/ctf-link.c    |  8 ++---
 libctf/libctf.ver    |  2 ++
 7 files changed, 68 insertions(+), 43 deletions(-)

-- 
2.29.0.249.g249b51256f

Comments

H.J. Lu via Binutils Nov. 17, 2020, 10:34 p.m. | #1
On Sun, Oct 25, 2020 at 02:14:03PM +0000, Nick Alcock via Binutils wrote:
> The functions that return ctf_dict_t's given a ctf_archive_t and a name

> are very clumsily named.  It sounds like they return *archives*, not

> dictionaries, and the names are very long and clunky.  Why do we

> have a ctf_arc_open_by_name when it opens a dictionary, not an archive,

> and when there is no way to open a dictionary in any other way?  The

> answer is purely internal: the function is located in ctf-archive.c,

> and everything in there was called ctf_arc_*, and there is another

> way to open a dict (by offset in the archive), that is internal to

> ctf-archive.c and that nothing else can call.

> 

> This is clearly bad naming. The internal organization of the source tree

> should not dictate public API names!

> 

> So rename things (keeping the old, bad names for compatibility), and

> adjust all users.  You now open a dict using ctf_dict_open, and

> open it giving ELF sections via ctf_dict_open_sections.


This is all fine too.

-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 54f4081d1c9..bf8234987f0 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4194,7 +4194,7 @@  dump_ctf (bfd *abfd, const char *sect_name, const char *parent_name)
   /* Assume that the applicable parent archive member is the default one.
      (This is what all known implementations are expected to do, if they
      put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_arc_open_by_name (lookparent, NULL, &err)) == NULL)
+  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       non_fatal (_("CTF open failure: %s"), ctf_errmsg (err));
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 5aec6f43bcd..8577d0d2212 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -14734,7 +14734,7 @@  dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
   /* Assume that the applicable parent archive member is the default one.
      (This is what all known implementations are expected to do, if they
      put CTFs and their parents in archives together.)  */
-  if ((parent = ctf_arc_open_by_name (lookparent, NULL, &err)) == NULL)
+  if ((parent = ctf_dict_open (lookparent, NULL, &err)) == NULL)
     {
       dump_ctf_errs (NULL);
       error (_("CTF open failure: %s\n"), ctf_errmsg (err));
diff --git a/gdb/ctfread.c b/gdb/ctfread.c
index 1f8ab942f08..8edfd5a8378 100644
--- a/gdb/ctfread.c
+++ b/gdb/ctfread.c
@@ -1482,9 +1482,9 @@  elfctf_build_psymtabs (struct objfile *of)
     error (_("ctf_bfdopen failed on %s - %s"),
 	   bfd_get_filename (abfd), ctf_errmsg (err));
 
-  ctf_dict_t *fp = ctf_arc_open_by_name (arc, NULL, &err);
+  ctf_dict_t *fp = ctf_dict_open (arc, NULL, &err);
   if (fp == NULL)
-    error (_("ctf_arc_open_by_name failed on %s - %s"),
+    error (_("ctf_dict_open failed on %s - %s"),
 	   bfd_get_filename (abfd), ctf_errmsg (err));
   ctf_dict_key.emplace (of, fp);
 
diff --git a/include/ctf-api.h b/include/ctf-api.h
index 60e06b39ee7..38d7758a48e 100644
--- a/include/ctf-api.h
+++ b/include/ctf-api.h
@@ -306,12 +306,12 @@  extern ctf_archive_t *ctf_arc_bufopen (const ctf_sect_t *,
 				       const ctf_sect_t *,
 				       int *);
 extern void ctf_arc_close (ctf_archive_t *);
-extern ctf_dict_t *ctf_arc_open_by_name (const ctf_archive_t *,
-					 const char *, int *);
-extern ctf_dict_t *ctf_arc_open_by_name_sections (const ctf_archive_t *,
-						  const ctf_sect_t *,
-						  const ctf_sect_t *,
-						  const char *, int *);
+extern ctf_dict_t *ctf_dict_open (const ctf_archive_t *,
+				  const char *, int *);
+extern ctf_dict_t *ctf_dict_open_sections (const ctf_archive_t *,
+					   const ctf_sect_t *,
+					   const ctf_sect_t *,
+					   const char *, int *);
 extern size_t ctf_archive_count (const ctf_archive_t *);
 
 /* The next functions return or close real CTF files, or write out CTF archives,
@@ -511,6 +511,12 @@  struct ctf_file;
 typedef struct ctf_dict ctf_file_t;
 extern void ctf_file_close (ctf_file_t *);
 extern ctf_dict_t *ctf_parent_file (ctf_dict_t *);
+extern ctf_dict_t *ctf_arc_open_by_name (const ctf_archive_t *,
+					 const char *, int *);
+extern ctf_dict_t *ctf_arc_open_by_name_sections (const ctf_archive_t *,
+						  const ctf_sect_t *,
+						  const ctf_sect_t *,
+						  const char *, int *);
 
 #ifdef	__cplusplus
 }
diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 90780da3834..72cdef91ac4 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -33,10 +33,10 @@ 
 #endif
 
 static off_t arc_write_one_ctf (ctf_dict_t * f, int fd, size_t threshold);
-static ctf_dict_t *ctf_arc_open_by_offset (const struct ctf_archive *arc,
-					   const ctf_sect_t *symsect,
-					   const ctf_sect_t *strsect,
-					   size_t offset, int *errp);
+static ctf_dict_t *ctf_dict_open_by_offset (const struct ctf_archive *arc,
+					    const ctf_sect_t *symsect,
+					    const ctf_sect_t *strsect,
+					    size_t offset, int *errp);
 static int sort_modent_by_name (const void *one, const void *two, void *n);
 static void *arc_mmap_header (int fd, size_t headersz);
 static void *arc_mmap_file (int fd, size_t size);
@@ -506,10 +506,10 @@  ctf_arc_close (ctf_archive_t *arc)
 /* Return the ctf_dict_t with the given name, or NULL if none, setting 'err' if
    non-NULL.  A name of NULL means to open the default file.  */
 static ctf_dict_t *
-ctf_arc_open_by_name_internal (const struct ctf_archive *arc,
-			       const ctf_sect_t *symsect,
-			       const ctf_sect_t *strsect,
-			       const char *name, int *errp)
+ctf_dict_open_internal (const struct ctf_archive *arc,
+			const ctf_sect_t *symsect,
+			const ctf_sect_t *strsect,
+			const char *name, int *errp)
 {
   struct ctf_archive_modent *modent;
   const char *search_nametbl;
@@ -517,7 +517,7 @@  ctf_arc_open_by_name_internal (const struct ctf_archive *arc,
   if (name == NULL)
     name = _CTF_SECTION;		 /* The default name.  */
 
-  ctf_dprintf ("ctf_arc_open_by_name(%s): opening\n", name);
+  ctf_dprintf ("ctf_dict_open_internal(%s): opening\n", name);
 
   modent = (ctf_archive_modent_t *) ((char *) arc
 				     + sizeof (struct ctf_archive));
@@ -536,8 +536,8 @@  ctf_arc_open_by_name_internal (const struct ctf_archive *arc,
       return NULL;
     }
 
-  return ctf_arc_open_by_offset (arc, symsect, strsect,
-				 le64toh (modent->ctf_offset), errp);
+  return ctf_dict_open_by_offset (arc, symsect, strsect,
+				  le64toh (modent->ctf_offset), errp);
 }
 
 /* Return the ctf_dict_t with the given name, or NULL if none, setting 'err' if
@@ -547,17 +547,17 @@  ctf_arc_open_by_name_internal (const struct ctf_archive *arc,
 
    Public entry point.  */
 ctf_dict_t *
-ctf_arc_open_by_name_sections (const ctf_archive_t *arc,
-			       const ctf_sect_t *symsect,
-			       const ctf_sect_t *strsect,
-			       const char *name,
-			       int *errp)
+ctf_dict_open_sections (const ctf_archive_t *arc,
+			const ctf_sect_t *symsect,
+			const ctf_sect_t *strsect,
+			const char *name,
+			int *errp)
 {
   if (arc->ctfi_is_archive)
     {
       ctf_dict_t *ret;
-      ret = ctf_arc_open_by_name_internal (arc->ctfi_archive, symsect, strsect,
-					   name, errp);
+      ret = ctf_dict_open_internal (arc->ctfi_archive, symsect, strsect,
+				    name, errp);
       if (ret)
 	ret->ctf_archive = (ctf_archive_t *) arc;
       return ret;
@@ -581,7 +581,7 @@  ctf_arc_open_by_name_sections (const ctf_archive_t *arc,
 
    Public entry point.  */
 ctf_dict_t *
-ctf_arc_open_by_name (const ctf_archive_t *arc, const char *name, int *errp)
+ctf_dict_open (const ctf_archive_t *arc, const char *name, int *errp)
 {
   const ctf_sect_t *symsect = &arc->ctfi_symsect;
   const ctf_sect_t *strsect = &arc->ctfi_strsect;
@@ -591,21 +591,21 @@  ctf_arc_open_by_name (const ctf_archive_t *arc, const char *name, int *errp)
   if (strsect->cts_name == NULL)
     strsect = NULL;
 
-  return ctf_arc_open_by_name_sections (arc, symsect, strsect, name, errp);
+  return ctf_dict_open_sections (arc, symsect, strsect, name, errp);
 }
 
 /* Return the ctf_dict_t at the given ctfa_ctfs-relative offset, or NULL if
    none, setting 'err' if non-NULL.  */
 static ctf_dict_t *
-ctf_arc_open_by_offset (const struct ctf_archive *arc,
-			const ctf_sect_t *symsect,
-			const ctf_sect_t *strsect, size_t offset,
-			int *errp)
+ctf_dict_open_by_offset (const struct ctf_archive *arc,
+			 const ctf_sect_t *symsect,
+			 const ctf_sect_t *strsect, size_t offset,
+			 int *errp)
 {
   ctf_sect_t ctfsect;
   ctf_dict_t *fp;
 
-  ctf_dprintf ("ctf_arc_open_by_offset(%lu): opening\n", (unsigned long) offset);
+  ctf_dprintf ("ctf_dict_open_by_offset(%lu): opening\n", (unsigned long) offset);
 
   memset (&ctfsect, 0, sizeof (ctf_sect_t));
 
@@ -621,6 +621,24 @@  ctf_arc_open_by_offset (const struct ctf_archive *arc,
   return fp;
 }
 
+/* Backward compatibility.  */
+ctf_dict_t *
+ctf_arc_open_by_name (const ctf_archive_t *arc, const char *name,
+		      int *errp)
+{
+  return ctf_dict_open (arc, name, errp);
+}
+
+ctf_dict_t *
+ctf_arc_open_by_name_sections (const ctf_archive_t *arc,
+			       const ctf_sect_t *symsect,
+			       const ctf_sect_t *strsect,
+			       const char *name,
+			       int *errp)
+{
+  return ctf_dict_open_sections (arc, symsect, strsect, name, errp);
+}
+
 /* Return the number of members in an archive.  */
 size_t
 ctf_archive_count (const ctf_archive_t *wrapper)
@@ -699,8 +717,8 @@  ctf_archive_iter_internal (const ctf_archive_t *wrapper,
       const char *name;
 
       name = &nametbl[le64toh (modent[i].name_offset)];
-      if ((f = ctf_arc_open_by_name_internal (arc, symsect, strsect,
-					      name, &rc)) == NULL)
+      if ((f = ctf_dict_open_internal (arc, symsect, strsect,
+				    name, &rc)) == NULL)
 	return rc;
 
       f->ctf_archive = (ctf_archive_t *) wrapper;
@@ -837,8 +855,7 @@  ctf_archive_next (const ctf_archive_t *wrapper, ctf_next_t **it, const char **na
   if (name)
     *name = name_;
 
-  f = ctf_arc_open_by_name_internal (arc, symsect, strsect,
-				     name_, errp);
+  f = ctf_dict_open_internal (arc, symsect, strsect, name_, errp);
   f->ctf_archive = (ctf_archive_t *) wrapper;
   return f;
 }
diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index c3801e6f3cb..192d4fd6622 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -789,8 +789,8 @@  ctf_link_one_input_archive (void *key, void *value, void *arg_)
 
   arg->in_file_name = file_name;
   arg->done_parent = 0;
-  if ((arg->in_fp_parent = ctf_arc_open_by_name (input->clin_arc, NULL,
-						 &err)) == NULL)
+  if ((arg->in_fp_parent = ctf_dict_open (input->clin_arc,
+					  NULL, &err)) == NULL)
     if (err != ECTF_ARNNAME)
       {
 	ctf_err_warn (arg->out_fp, 1, 0,
@@ -1015,8 +1015,8 @@  ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
 	 multiple members.  We assume, as elsewhere, that the parent is named
 	 _CTF_SECTION.  */
 
-      if ((parent_fp = ctf_arc_open_by_name (one_input->clin_arc,
-					     _CTF_SECTION, &err)) == NULL)
+      if ((parent_fp = ctf_dict_open (one_input->clin_arc, _CTF_SECTION,
+				      &err)) == NULL)
 	{
 	  if (err != ECTF_NOMEMBNAM)
 	    {
diff --git a/libctf/libctf.ver b/libctf/libctf.ver
index 5f5dc57885d..cf4fa7396b5 100644
--- a/libctf/libctf.ver
+++ b/libctf/libctf.ver
@@ -177,6 +177,8 @@  LIBCTF_1.0 {
 
 LIBCTF_1.1 {
     global:
+	ctf_dict_open;
+	ctf_dict_open_sections;
 	ctf_dict_close;
 	ctf_parent_dict;
 } LIBCTF_1.0;