Re: [PATCH] Small segfault fix when there is no python

[Doug Evans <dje@google.com>]

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!