Re: [PATCH v2 1/2] gdb: Require psymtab before calling quick_functions in objfile

[Lancelot SIX <Lancelot.Six@amd.com>]

On 20/05/2022 16:51, Tom Tromey wrote:
> [CAUTION: External Email]
> 
>>>>>> "Lancelot" == Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Lancelot> we see something like:
> Lancelot> This patch proposes to wrap the access of the `qf` field with an accessor
> Lancelot> which ensures that partial symbols have been read before iterating:
> Lancelot> qf_require_partial_symbols.  All calls to quick functions are updated
> Lancelot> except:
> 
> Thank you for the patch.
> I have one small nit.
> 
> Lancelot> +  /* Ensure that partial symbols have been read and return the "quick" (aka
> Lancelot> +     partial) symbol functions for this symbol reader.  */
> Lancelot> +
> Lancelot> +  const std::forward_list<quick_symbol_functions_up> &
> Lancelot> +    qf_require_partial_symbols ()
> Lancelot> +      {
> Lancelot> +     this->require_partial_symbols (true);
> Lancelot> +     return qf;
> Lancelot> +      }
> Lancelot> +
> 
> It seems like this could be private instead of public.
> 
> It's ok with that change, you don't need to re-send it.
> 
> Tom

Hi,

Thanks for the review.

I have changed the declaration locally as follows (I did some back and 
forth on whether it should stay grouped with the other methods of 
ojbfile bracketed between a pritave:/public: pair or if it should go in 
a private section at the end of the struct — I finally opted for former).

I'll keep this and push it with the revised version of patch #2 once 
approved.

Best,
Lancelot.

---

diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index cb7a1357cfe..9da12ff12e0 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -612,6 +612,19 @@ struct objfile
      this->section_offsets[idx] = offset;
    }

+private:
+
+  /* Ensure that partial symbols have been read and return the "quick" (aka
+     partial) symbol functions for this symbol reader.  */
+  const std::forward_list<quick_symbol_functions_up> &
+  qf_require_partial_symbols ()
+  {
+    this->require_partial_symbols (true);
+    return qf;
+  }
+
+public:
+
    /* The object file's original name as specified by the user,
       made absolute, and tilde-expanded.  However, it is not canonicalized
       (i.e., it has not been passed through gdb_realpath).