Commit Message
I stumbled upon a few comments that I think are outdated.
Comment for elfread.c (elf_symfile_init): As far as history goes in git,
I don't see anything related to that.
Comment for elfread.c (elf_symfile_read): References a parameter that was
removed in 1999.
Comment for struct sym_fns/sym_offsets: References a parameter that was
changed in 1999.
gdb/ChangeLog:
* elfread.c (elf_symfile_init): Remove stale comment.
(elf_symfile_read): Same.
* symfile.h (struct sym_fns): Same.
---
gdb/elfread.c | 13 +------------
gdb/symfile.h | 6 +-----
2 files changed, 2 insertions(+), 17 deletions(-)
Comments
On Wed, Dec 17, 2014 at 11:01 AM, Simon Marchi
<simon.marchi@ericsson.com> wrote:
> I stumbled upon a few comments that I think are outdated.
>
> Comment for elfread.c (elf_symfile_init): As far as history goes in git,
> I don't see anything related to that.
>
> Comment for elfread.c (elf_symfile_read): References a parameter that was
> removed in 1999.
>
> Comment for struct sym_fns/sym_offsets: References a parameter that was
> changed in 1999.
>
> gdb/ChangeLog:
>
> * elfread.c (elf_symfile_init): Remove stale comment.
> (elf_symfile_read): Same.
> * symfile.h (struct sym_fns): Same.
Hi.
Ok with one nit, below.
> ---
> gdb/elfread.c | 13 +------------
> gdb/symfile.h | 6 +-----
> 2 files changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index b4ec067..93e5045 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -1236,10 +1236,6 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
> We have been initialized by a call to elf_symfile_init, which
> currently does nothing.
>
> - SECTION_OFFSETS is a set of offsets to apply to relocate the symbols
> - in each section. We simplify it down to a single offset for all
> - symbols. FIXME.
> -
> This function only does the minimum work necessary for letting the
> user "name" things symbolically; it does not read the entire symtab.
> Instead, it reads the external and static symbols and puts them in partial
> @@ -1418,14 +1414,7 @@ elf_symfile_finish (struct objfile *objfile)
> dwarf2_free_objfile (objfile);
> }
>
> -/* ELF specific initialization routine for reading symbols.
> -
> - It is passed a pointer to a struct sym_fns which contains, among other
> - things, the BFD for the file whose symbols are being read, and a slot for
> - a pointer to "private data" which we can fill with goodies.
> -
> - For now at least, we have nothing in particular to do, so this function is
> - just a stub. */
> +/* ELF specific initialization routine for reading symbols. */
>
> static void
> elf_symfile_init (struct objfile *objfile)
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index 1e8c230..b2cb089 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -354,11 +354,7 @@ struct sym_fns
> void (*sym_finish) (struct objfile *);
>
> /* This function produces a file-dependent section_offsets
> - structure, allocated in the objfile's storage, and based on the
> - parameter. The parameter is currently a CORE_ADDR (FIXME!) for
> - backward compatibility with the higher levels of GDB. It should
> - probably be changed to a string, where NULL means the default,
> - and others are parsed in a file dependent way. */
> + structure, allocated in the objfile's storage. */
>
> void (*sym_offsets) (struct objfile *, const struct section_addr_info *);
Can I ask you to document what the struct section_addr_info * argument is for?
On 2014-12-17 02:37 PM, Doug Evans wrote:
> On Wed, Dec 17, 2014 at 11:01 AM, Simon Marchi
> <simon.marchi@ericsson.com> wrote:
>> I stumbled upon a few comments that I think are outdated.
>>
>> Comment for elfread.c (elf_symfile_init): As far as history goes in git,
>> I don't see anything related to that.
>>
>> Comment for elfread.c (elf_symfile_read): References a parameter that was
>> removed in 1999.
>>
>> Comment for struct sym_fns/sym_offsets: References a parameter that was
>> changed in 1999.
>>
>> gdb/ChangeLog:
>>
>> * elfread.c (elf_symfile_init): Remove stale comment.
>> (elf_symfile_read): Same.
>> * symfile.h (struct sym_fns): Same.
>
> Hi.
> Ok with one nit, below.
>
>> ---
>> gdb/elfread.c | 13 +------------
>> gdb/symfile.h | 6 +-----
>> 2 files changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/gdb/elfread.c b/gdb/elfread.c
>> index b4ec067..93e5045 100644
>> --- a/gdb/elfread.c
>> +++ b/gdb/elfread.c
>> @@ -1236,10 +1236,6 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
>> We have been initialized by a call to elf_symfile_init, which
>> currently does nothing.
>>
>> - SECTION_OFFSETS is a set of offsets to apply to relocate the symbols
>> - in each section. We simplify it down to a single offset for all
>> - symbols. FIXME.
>> -
>> This function only does the minimum work necessary for letting the
>> user "name" things symbolically; it does not read the entire symtab.
>> Instead, it reads the external and static symbols and puts them in partial
>> @@ -1418,14 +1414,7 @@ elf_symfile_finish (struct objfile *objfile)
>> dwarf2_free_objfile (objfile);
>> }
>>
>> -/* ELF specific initialization routine for reading symbols.
>> -
>> - It is passed a pointer to a struct sym_fns which contains, among other
>> - things, the BFD for the file whose symbols are being read, and a slot for
>> - a pointer to "private data" which we can fill with goodies.
>> -
>> - For now at least, we have nothing in particular to do, so this function is
>> - just a stub. */
>> +/* ELF specific initialization routine for reading symbols. */
>>
>> static void
>> elf_symfile_init (struct objfile *objfile)
>> diff --git a/gdb/symfile.h b/gdb/symfile.h
>> index 1e8c230..b2cb089 100644
>> --- a/gdb/symfile.h
>> +++ b/gdb/symfile.h
>> @@ -354,11 +354,7 @@ struct sym_fns
>> void (*sym_finish) (struct objfile *);
>>
>> /* This function produces a file-dependent section_offsets
>> - structure, allocated in the objfile's storage, and based on the
>> - parameter. The parameter is currently a CORE_ADDR (FIXME!) for
>> - backward compatibility with the higher levels of GDB. It should
>> - probably be changed to a string, where NULL means the default,
>> - and others are parsed in a file dependent way. */
>> + structure, allocated in the objfile's storage. */
>>
>> void (*sym_offsets) (struct objfile *, const struct section_addr_info *);
>
> Can I ask you to document what the struct section_addr_info * argument is for?
>
Hmm I am not sure my knowledge allows me to produce a useful comment. Don't hesitate to
correct/enhance. Here is my first shot, after a bit of code reading:
/* This function produces a file-dependent section_offsets
structure, allocated in the objfile's storage.
The section_addr_info structure contains the offset of loadable and
allocated sections, relative to the absolute offsets found in the BFD. */
void (*sym_offsets) (struct objfile *, const struct section_addr_info *);
On Wed, Dec 17, 2014 at 1:45 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
> On 2014-12-17 02:37 PM, Doug Evans wrote:
>> On Wed, Dec 17, 2014 at 11:01 AM, Simon Marchi
>> <simon.marchi@ericsson.com> wrote:
>>> I stumbled upon a few comments that I think are outdated.
>>>
>>> Comment for elfread.c (elf_symfile_init): As far as history goes in git,
>>> I don't see anything related to that.
>>>
>>> Comment for elfread.c (elf_symfile_read): References a parameter that was
>>> removed in 1999.
>>>
>>> Comment for struct sym_fns/sym_offsets: References a parameter that was
>>> changed in 1999.
>>>
>>> gdb/ChangeLog:
>>>
>>> * elfread.c (elf_symfile_init): Remove stale comment.
>>> (elf_symfile_read): Same.
>>> * symfile.h (struct sym_fns): Same.
>>
>> Hi.
>> Ok with one nit, below.
>>
>>> ---
>>> gdb/elfread.c | 13 +------------
>>> gdb/symfile.h | 6 +-----
>>> 2 files changed, 2 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/gdb/elfread.c b/gdb/elfread.c
>>> index b4ec067..93e5045 100644
>>> --- a/gdb/elfread.c
>>> +++ b/gdb/elfread.c
>>> @@ -1236,10 +1236,6 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
>>> We have been initialized by a call to elf_symfile_init, which
>>> currently does nothing.
>>>
>>> - SECTION_OFFSETS is a set of offsets to apply to relocate the symbols
>>> - in each section. We simplify it down to a single offset for all
>>> - symbols. FIXME.
>>> -
>>> This function only does the minimum work necessary for letting the
>>> user "name" things symbolically; it does not read the entire symtab.
>>> Instead, it reads the external and static symbols and puts them in partial
>>> @@ -1418,14 +1414,7 @@ elf_symfile_finish (struct objfile *objfile)
>>> dwarf2_free_objfile (objfile);
>>> }
>>>
>>> -/* ELF specific initialization routine for reading symbols.
>>> -
>>> - It is passed a pointer to a struct sym_fns which contains, among other
>>> - things, the BFD for the file whose symbols are being read, and a slot for
>>> - a pointer to "private data" which we can fill with goodies.
>>> -
>>> - For now at least, we have nothing in particular to do, so this function is
>>> - just a stub. */
>>> +/* ELF specific initialization routine for reading symbols. */
>>>
>>> static void
>>> elf_symfile_init (struct objfile *objfile)
>>> diff --git a/gdb/symfile.h b/gdb/symfile.h
>>> index 1e8c230..b2cb089 100644
>>> --- a/gdb/symfile.h
>>> +++ b/gdb/symfile.h
>>> @@ -354,11 +354,7 @@ struct sym_fns
>>> void (*sym_finish) (struct objfile *);
>>>
>>> /* This function produces a file-dependent section_offsets
>>> - structure, allocated in the objfile's storage, and based on the
>>> - parameter. The parameter is currently a CORE_ADDR (FIXME!) for
>>> - backward compatibility with the higher levels of GDB. It should
>>> - probably be changed to a string, where NULL means the default,
>>> - and others are parsed in a file dependent way. */
>>> + structure, allocated in the objfile's storage. */
>>>
>>> void (*sym_offsets) (struct objfile *, const struct section_addr_info *);
>>
>> Can I ask you to document what the struct section_addr_info * argument is for?
>>
>
> Hmm I am not sure my knowledge allows me to produce a useful comment. Don't hesitate to
> correct/enhance. Here is my first shot, after a bit of code reading:
>
> /* This function produces a file-dependent section_offsets
> structure, allocated in the objfile's storage.
>
> The section_addr_info structure contains the offset of loadable and
> allocated sections, relative to the absolute offsets found in the BFD. */
>
> void (*sym_offsets) (struct objfile *, const struct section_addr_info *);
"works for me"
Thanks!
@@ -1236,10 +1236,6 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
We have been initialized by a call to elf_symfile_init, which
currently does nothing.
- SECTION_OFFSETS is a set of offsets to apply to relocate the symbols
- in each section. We simplify it down to a single offset for all
- symbols. FIXME.
-
This function only does the minimum work necessary for letting the
user "name" things symbolically; it does not read the entire symtab.
Instead, it reads the external and static symbols and puts them in partial
@@ -1418,14 +1414,7 @@ elf_symfile_finish (struct objfile *objfile)
dwarf2_free_objfile (objfile);
}
-/* ELF specific initialization routine for reading symbols.
-
- It is passed a pointer to a struct sym_fns which contains, among other
- things, the BFD for the file whose symbols are being read, and a slot for
- a pointer to "private data" which we can fill with goodies.
-
- For now at least, we have nothing in particular to do, so this function is
- just a stub. */
+/* ELF specific initialization routine for reading symbols. */
static void
elf_symfile_init (struct objfile *objfile)
@@ -354,11 +354,7 @@ struct sym_fns
void (*sym_finish) (struct objfile *);
/* This function produces a file-dependent section_offsets
- structure, allocated in the objfile's storage, and based on the
- parameter. The parameter is currently a CORE_ADDR (FIXME!) for
- backward compatibility with the higher levels of GDB. It should
- probably be changed to a string, where NULL means the default,
- and others are parsed in a file dependent way. */
+ structure, allocated in the objfile's storage. */
void (*sym_offsets) (struct objfile *, const struct section_addr_info *);