This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Don't check PST is NULL in read_symtab
On 01/11/2013 03:05 PM, Joel Brobecker wrote:
>> IMO, we don't need an assertion to check PST, because the function is
>> used in this way,
>>
>> (*pst->read_symtab) (objfile, pst);
>
> I am fine without the assertion as well. But if we followed your
> argument, we would never need an assertion. For me, assertions
> achieve two goals:
> 1. Clearly document an expectation;
> 2. Cause a semi-friendly abortion, rather than a mysterious behavior
> or crash.
> As of today, the way this function is called indeed guarantees that
> PST is never NULL. But someone adding a call at a later date might
> introduce a bug and cause it to be called with PST == NULL...
FWIW, I agree with both of you. I agree with assertion's
roles. But I also agree with Yao that for functions that implement
a class-like interface and take a "this" pointer, there's no need
to sprinkle the codebase with "gdb_assert (self != NULL)" checks.
BUT (!), when reading one of those functions, it's a bit more obvious
and self-describing that the function takes a "this"-style pointer
when the parameter is actually called "self", and / or at least is
the first parameter in the function's signature. Like:
static void
dbx_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
{
static void
dbx_psymtab_to_symtab_1 (struct partial_symtab *pst, struct objfile *objfile)
{
static void
dbx_psymtab_to_symtab_1 (struct partial_symtab *self, struct objfile *objfile)
{
(It'd be even better for grepability/readability if the implementations
and hook name agreed, like:
- result->read_symtab = dbx_psymtab_to_symtab;
+ result->read_symtab = dbx_read_symtab;
or
- result->read_symtab = dbx_psymtab_to_symtab;
+ result->psymtab_to_symtab = dbx_psymtab_to_symtab;
...
)
--
Pedro Alves