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
> 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:
I agree with everything said above. Just to be extra clear, in
case I appeared to be insisting - I was just thinking about
the assert, not requesting it.
> 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)
> {
I like "self" quite a bit, except that it's a little bit less
descriptive that "pst". But for a "method", I think that's
acceptable.
> (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;
Agree as well. I don't mind making these two changes after Yao's
commit goes it (unless Yao wants to take care of it, I don't mean
to cut in).
--
Joel