Message ID | CAF5HaEU+9uQn=P37F=c8WxausmA3BPL9SKXoTVZh+2SNA1V1cw@mail.gmail.com |
---|---|
State | Committed |
Headers | show |
On Thursday, March 20 2014, Daniel Gutson wrote: > Hi, > > the small attached patch prevents gdb to segfault when an extension > language definition has no ops, > which e.g. occurs when HAVE_PYTHON is not defined so > extension_language_python remains with ops in NULL. > This causes the line > if (extlang->ops->eval_from_control_command != NULL) > (in eval_ext_lang_from_control_command) to dereference a null pointer. Hi Daniel, Thanks for the patch. This is a simple patch so it doesn't need a copyright assignment from you. However, if you intend to continue contributing to GDB, please e-mail me offlist and I can send you the papers to obtain the assignment. Just a few comments about formatting issues. > 2014-03-20 Daniel Gutson (daniel.gutson@tallertechnologies.com) > > gdb/ > * extension.c: (eval_ext_lang_from_control_command) Added check to > prevent dereference of null pointer. The ChangeLog format is wrong. Take a look at the ChangeLog file for lots of examples, but basically you need to write: 2014-03-20 Your Name <your@email> * file.c (function): Added check to prevent blabla... Pay attention to the 2 spaces between the date, the name and the e-mail, and also to the TAB character indenting the description. > diff --git a/gdb/extension.c b/gdb/extension.c > index c2f502b..8357ee8 100644 > --- a/gdb/extension.c > +++ b/gdb/extension.c > @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd) > { > if (extlang->cli_control_type == cmd->control_type) > { > - if (extlang->ops->eval_from_control_command != NULL) > - { > - extlang->ops->eval_from_control_command (extlang, cmd); > - return; > - } > + if (extlang->ops != NULL) > + { > + if (extlang->ops->eval_from_control_command != NULL) > + { > + extlang->ops->eval_from_control_command (extlang, cmd); > + return; > + } > + } You could simplify this by writing: if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL) If you don't want to join the two "if"s, then you don't need to put the braces on the outter "if", because it has one single statement. The patch looks good to me, but I'm not a maintainer and can't approve it. Thanks,
On Thu, Mar 20, 2014 at 10:31 AM, Sergio Durigan Junior <sergiodj@redhat.com> wrote: > On Thursday, March 20 2014, Daniel Gutson wrote: > >> Hi, >> >> the small attached patch prevents gdb to segfault when an extension >> language definition has no ops, >> which e.g. occurs when HAVE_PYTHON is not defined so >> extension_language_python remains with ops in NULL. >> This causes the line >> if (extlang->ops->eval_from_control_command != NULL) >> (in eval_ext_lang_from_control_command) to dereference a null pointer. > > Hi Daniel, > > Thanks for the patch. This is a simple patch so it doesn't need a > copyright assignment from you. However, if you intend to continue > contributing to GDB, please e-mail me offlist and I can send you the > papers to obtain the assignment. > > Just a few comments about formatting issues. > >> 2014-03-20 Daniel Gutson (daniel.gutson@tallertechnologies.com) >> >> gdb/ >> * extension.c: (eval_ext_lang_from_control_command) Added check to >> prevent dereference of null pointer. > > The ChangeLog format is wrong. Take a look at the ChangeLog file for > lots of examples, but basically you need to write: > > 2014-03-20 Your Name <your@email> > > * file.c (function): Added check to prevent blabla... > > Pay attention to the 2 spaces between the date, the name and the e-mail, > and also to the TAB character indenting the description. > >> diff --git a/gdb/extension.c b/gdb/extension.c >> index c2f502b..8357ee8 100644 >> --- a/gdb/extension.c >> +++ b/gdb/extension.c >> @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd) >> { >> if (extlang->cli_control_type == cmd->control_type) >> { >> - if (extlang->ops->eval_from_control_command != NULL) >> - { >> - extlang->ops->eval_from_control_command (extlang, cmd); >> - return; >> - } >> + if (extlang->ops != NULL) >> + { >> + if (extlang->ops->eval_from_control_command != NULL) >> + { >> + extlang->ops->eval_from_control_command (extlang, cmd); >> + return; >> + } >> + } > > You could simplify this by writing: > > if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL) > > If you don't want to join the two "if"s, then you don't need to put the > braces on the outter "if", because it has one single statement. > > The patch looks good to me, but I'm not a maintainer and can't approve > it. > > Thanks, > > -- > Sergio I did an audit of all the uses of ->ops and think this is the only one I missed. I will commit with the needed changes. Thanks for the patch!
Thanks Doug and Sergio! Daniel. On Thu, Mar 20, 2014 at 2:52 PM, Doug Evans <dje@google.com> wrote: > On Thu, Mar 20, 2014 at 10:31 AM, Sergio Durigan Junior > <sergiodj@redhat.com> wrote: >> On Thursday, March 20 2014, Daniel Gutson wrote: >> >>> Hi, >>> >>> the small attached patch prevents gdb to segfault when an extension >>> language definition has no ops, >>> which e.g. occurs when HAVE_PYTHON is not defined so >>> extension_language_python remains with ops in NULL. >>> This causes the line >>> if (extlang->ops->eval_from_control_command != NULL) >>> (in eval_ext_lang_from_control_command) to dereference a null pointer. >> >> Hi Daniel, >> >> Thanks for the patch. This is a simple patch so it doesn't need a >> copyright assignment from you. However, if you intend to continue >> contributing to GDB, please e-mail me offlist and I can send you the >> papers to obtain the assignment. >> >> Just a few comments about formatting issues. >> >>> 2014-03-20 Daniel Gutson (daniel.gutson@tallertechnologies.com) >>> >>> gdb/ >>> * extension.c: (eval_ext_lang_from_control_command) Added check to >>> prevent dereference of null pointer. >> >> The ChangeLog format is wrong. Take a look at the ChangeLog file for >> lots of examples, but basically you need to write: >> >> 2014-03-20 Your Name <your@email> >> >> * file.c (function): Added check to prevent blabla... >> >> Pay attention to the 2 spaces between the date, the name and the e-mail, >> and also to the TAB character indenting the description. >> >>> diff --git a/gdb/extension.c b/gdb/extension.c >>> index c2f502b..8357ee8 100644 >>> --- a/gdb/extension.c >>> +++ b/gdb/extension.c >>> @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd) >>> { >>> if (extlang->cli_control_type == cmd->control_type) >>> { >>> - if (extlang->ops->eval_from_control_command != NULL) >>> - { >>> - extlang->ops->eval_from_control_command (extlang, cmd); >>> - return; >>> - } >>> + if (extlang->ops != NULL) >>> + { >>> + if (extlang->ops->eval_from_control_command != NULL) >>> + { >>> + extlang->ops->eval_from_control_command (extlang, cmd); >>> + return; >>> + } >>> + } >> >> You could simplify this by writing: >> >> if (extlang->ops != NULL && extlang->ops->eval_from_control_command != NULL) >> >> If you don't want to join the two "if"s, then you don't need to put the >> braces on the outter "if", because it has one single statement. >> >> The patch looks good to me, but I'm not a maintainer and can't approve >> it. >> >> Thanks, >> >> -- >> Sergio > > > I did an audit of all the uses of ->ops and think this is the only one I missed. > I will commit with the needed changes. > Thanks for the patch!
diff --git a/gdb/extension.c b/gdb/extension.c index c2f502b..8357ee8 100644 --- a/gdb/extension.c +++ b/gdb/extension.c @@ -342,11 +342,14 @@ eval_ext_lang_from_control_command (struct command_line *cmd) { if (extlang->cli_control_type == cmd->control_type) { - if (extlang->ops->eval_from_control_command != NULL) - { - extlang->ops->eval_from_control_command (extlang, cmd); - return; - } + if (extlang->ops != NULL) + { + if (extlang->ops->eval_from_control_command != NULL) + { + extlang->ops->eval_from_control_command (extlang, cmd); + return; + } + } /* The requested extension language is not supported in this GDB. */ throw_ext_lang_unsupported (extlang); }