[PATCH] A few comment cleanups

[PATCH] A few comment cleanups

[Simon Marchi]

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(-)

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 *);
 
-- 
2.1.3

Re: [PATCH] A few comment cleanups

[Doug Evans]

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?

Re: [PATCH] A few comment cleanups

[Simon Marchi]

[-- Attachment #1: Type: text/plain, Size: 3696 bytes --]

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 *);




[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4079 bytes --]

Re: [PATCH] A few comment cleanups

[Doug Evans]

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!

Re: [PATCH] A few comment cleanups

[Simon Marchi]

[-- Attachment #1: Type: text/plain, Size: 7611 bytes --]

On 2014-12-17 05:30 PM, Doug Evans wrote:
> 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!

Pushed with that change, thanks!

From db7a9bcd53534a32abf4d75a93838c6bdbf876fa Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 18 Dec 2014 11:39:44 -0500
Subject: [PATCH] A few comment cleanups

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/ChangeLog |  6 ++++++
 gdb/elfread.c | 13 +------------
 gdb/symfile.h | 10 +++++-----
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 348a48f..adb24d4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-12-18  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* elfread.c (elf_symfile_init): Remove stale comment.
+	(elf_symfile_read): Same.
+	* symfile.h (struct sym_fns): Same.
+
 2014-12-18  Nigel Stephens  <nigel@mips.com>
 	    Maciej W. Rozycki  <macro@codesourcery.com>

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..081bc4e 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -353,12 +353,12 @@ 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.
+
+     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 *);

-- 
2.1.3




[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4079 bytes --]