Re: [python][patch] Inferior and Thread information support.

[Phil Muldoon <pmuldoon at redhat dot com>]

On 06/15/2010 07:11 PM, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> I've no complaint to using obstacks.  This function basically
> Phil> wraps/tidies the existing code that was just coded directly in a loop
> Phil> in parse_find_args.  That code just realloc'd by a factor of two whenever
> Phil> the buffer was too small.  This code is exactly the same, except it
> Phil> has been squirrelled away in a function.  So we are not introducing or
> Phil> adding any more growable types in this patch, just moving the code
> Phil> bits that already existed into function.  I'm not adverse to changing
> Phil> that code to use obstacks, that being said!
> 
> Just for the record -- ordinarily I try not to request cleanups to
> existing code as part of a new patch.  It is nice to get cleanups, and
> if you want to do them (or if there is a reason for them beyond mere
> tidiness) then that is great.  But feel free to push back if I've
> erroneously reviewed the context and not the patch.


We talked a little about this on irc.  I think your original instincts
were totally right.  I had second thoughts about it.  We're going through this
bit of code, so lets just fix it -- and I'm happy to do it.  I did a
basic conversion to obstacks today; I'll refine it for the patch
later.  I don't see it being a huge issue. 

> 
> Phil> +  /* Find inferior_object for the given PID.  */
> Phil> +  for (inf_entry = &gdbpy_inferior_list; *inf_entry != NULL;
> Phil> +       inf_entry = &(*inf_entry)->next)
> Phil> +    if ((*inf_entry)->inf_obj->inferior->pid == inf->pid)
> Phil> +      break;
>>
> Tom> It seems strange to compare the pid fields when we could just compare
> Tom> the inferior objects themselves.
> 
> Phil> Do you mean using the Python object's cmp inbuilt method here?
> 
> No, I'm just curious why that can't be more simply written:
> 
>   if ((*inf_entry)->inf_obj == inf)


When I ported this code from Archer I thought the (t)pid equality was
fine.  But as in any porting effort you take the sum of several
authors and decide to rewrite/port what you have.  I thought that this
equality was ok.  OTOH I don't see why either (on your original
question).  I'll investigate further and report back.

Cheers,

Phil