* [PATCH] A few comment cleanups @ 2014-12-17 19:01 Simon Marchi 2014-12-17 19:37 ` Doug Evans 0 siblings, 1 reply; 5+ messages in thread From: Simon Marchi @ 2014-12-17 19:01 UTC (permalink / raw) To: gdb-patches; +Cc: 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] A few comment cleanups 2014-12-17 19:01 [PATCH] A few comment cleanups Simon Marchi @ 2014-12-17 19:37 ` Doug Evans 2014-12-17 21:45 ` Simon Marchi 0 siblings, 1 reply; 5+ messages in thread From: Doug Evans @ 2014-12-17 19:37 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches 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? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] A few comment cleanups 2014-12-17 19:37 ` Doug Evans @ 2014-12-17 21:45 ` Simon Marchi 2014-12-17 22:30 ` Doug Evans 0 siblings, 1 reply; 5+ messages in thread From: Simon Marchi @ 2014-12-17 21:45 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] A few comment cleanups 2014-12-17 21:45 ` Simon Marchi @ 2014-12-17 22:30 ` Doug Evans 2014-12-18 18:26 ` Simon Marchi 0 siblings, 1 reply; 5+ messages in thread From: Doug Evans @ 2014-12-17 22:30 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches 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! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] A few comment cleanups 2014-12-17 22:30 ` Doug Evans @ 2014-12-18 18:26 ` Simon Marchi 0 siblings, 0 replies; 5+ messages in thread From: Simon Marchi @ 2014-12-18 18:26 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-18 18:26 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-12-17 19:01 [PATCH] A few comment cleanups Simon Marchi 2014-12-17 19:37 ` Doug Evans 2014-12-17 21:45 ` Simon Marchi 2014-12-17 22:30 ` Doug Evans 2014-12-18 18:26 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox