This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA v2 06/17] Record minimal symbols directly in reader.
On 10/13/2016 10:10 PM, Tom Tromey wrote:
> This patch changes minimal symbol creation in two ways. First, it
> removes global variables in favor of members of minimal_symbol_reader.
> Second, it changes functions like prim_record_minimal_symbol to be
> member functions of minimal_symbol_reader.
>
Awesome, thanks.
> struct minimal_symbol *
> -prim_record_minimal_symbol_full (const char *name, int name_len, int copy_name,
> - CORE_ADDR address,
> - enum minimal_symbol_type ms_type,
> - int section,
> - struct objfile *objfile)
> +minimal_symbol_reader::record_full (const char *name, int name_len,
> + int copy_name,
Can you make this bool while at it?
> + CORE_ADDR address,
> + enum minimal_symbol_type ms_type,
> + int section)
> explicit minimal_symbol_reader (struct objfile *);
>
> @@ -73,6 +74,56 @@ class minimal_symbol_reader
>
> void install ();
>
> + /* Record a new minimal symbol. This is the "full" entry point;
> + simpler convenience entry points are also provided below.
> +
> + This returns a new minimal symbol. It is ok to modify the returned
> + minimal symbol (though generally not necessary). It is not ok,
> + though, to stash the pointer anywhere; as minimal symbols may be
> + moved after creation. The memory for the returned minimal symbol
> + is still owned by the minsyms.c code, and should not be freed.
> +
> + Arguments are:
> +
> + NAME - the symbol's name
> + NAME_LEN - the length of the name
> + COPY_NAME - if true, the minsym code must make a copy of NAME. If
> + false, then NAME must be NUL-terminated, and must have a lifetime
> + that is at least as long as OBJFILE's lifetime.
> + ADDRESS - the address of the symbol
> + MS_TYPE - the type of the symbol
> + SECTION - the symbol's section
> + appropriate obj_section for the minimal symbol. This can be NULL.
Hmm, preexisting, but, this parameter's type is actually int.
Not sure that comment makes sense as is.
> + OBJFILE - the objfile associated with the minimal symbol. */
ENOSUCHPARAMETER.
> +
> + struct minimal_symbol *record_full (const char *name,
> + int name_len,
> + int copy_name,
Use "bool" for copy_name ?
> + CORE_ADDR address,
> + enum minimal_symbol_type ms_type,
> + int section);
> +
> + /* Like record_full, but:
> + - uses strlen to compute NAME_LEN,
> + - passes COPY_NAME = 1,
"= true".
> + - and passes a default SECTION, depending on the type
> +
> + This variant does not return the new symbol. */
> +
> + void record (const char *, CORE_ADDR, enum minimal_symbol_type);
I think it may be a good idea to give names to the parameters.
> +
> + /* Like record_full, but:
> + - uses strlen to compute NAME_LEN,
> + - passes COPY_NAME = 1. */
= true.
> +
> + struct minimal_symbol *record_with_info (const char *name,
> + CORE_ADDR address,
> + enum minimal_symbol_type ms_type,
> + int section)
> + {
> + return record_full (name, strlen (name), 1, address, ms_type, section);
s/1/true/.
LGTM with those changes.
Thanks,
Pedro Alves