Re: [Qemu-devel] [PATCH] Remove unneeded function parameter from gen_pc_load

[Stefan Weil <weil@mail.berlios.de>]

Am 17.04.2011 23:43, schrieb Aurelien Jarno:
> On Sun, Apr 17, 2011 at 10:34:47PM +0100, Peter Maydell wrote:
>> On 17 April 2011 22:07, Stefan Weil <weil@mail.berlios.de> wrote:
>>> Am 17.04.2011 20:27, schrieb Aurelien Jarno:
>>>>
>>>> On Thu, Apr 14, 2011 at 08:50:00PM +0200, Stefan Weil wrote:
>>>>>
>>>>> Am 13.04.2011 23:05, schrieb Peter Maydell:
>>>>>>
>>>>>> On 13 April 2011 21:38, Stefan Weil <weil@mail.berlios.de> wrote:
>>>>>>>
>>>>>>> gen_pc_load was introduced in commit
>>>>>>> d2856f1ad4c259e5766847c49acbb4e390731bd4.
>>>>>>> The only reason for parameter searched_pc was
>>>>>>> a debug statement in target-i386/translate.c.
>>>>>>>
>>>>>>> Remove searched_pc from the debug statement
>>>>>>> and from the parameter list of gen_pc_load.
>>>>>>
>>>>>> No issues with the meat of the patch, but if we're going to
>>>>>> change all the callers and implementations of this anyway,
>>>>>> is there any appetite for giving it a more appropriate name?
>>>>>> It doesn't generate any code, it affects more than just the
>>>>>> pc, and it doesn't do a load...
>>>>>>
>>>>>> restore_state_to_opc() ? set_env_for_opc() ?
>>>>>>
>>>>>> -- PMM
>>>>>
>>>>>
>>>>> What about cpu_restore_pc()? That's not always the whole truth,
>>>>> but it's always the main action done in function n.n. which currently
>>>>> is called gen_pc_load.
>>>>>
>>>>> Or cpu_restore_helper()? Helper is very generic - it always fits.
>>>>>
>>>>> Aurelien, please feel free to choose a name which suits bests.
>>>>> I don't mind if you simply patch my patch, create a new one
>>>>> or tell me which name should go into a new version of the patch
>>>>> so I can send it.
>>>>>
>>>>
>>>> As Peter said, the function is doing more than simply restoring the
>>>> pc. I am fine with the name he proposed, I think restore_state_to_opc()
>>>> is a bit better.
>>>
>>> Ok, so I'll send a new patch which also replaces gen_pc_load
>>> by restore_state_to_op.
>>
>> That's _to_opc, not _to_op : I was trying to be consistent with
>> the naming of the gen_opc_* arrays.
>>
>
> Oops, sorry, just a cut & paste mistake.

I agree, but it was my mistake.

The first of the following patches is an improved version of my previous 
patch.
It uses the correct name restore_state_to_opc and also removes a second 
parameter.

The second patch is based on the first and removes a parameter
from another function.

[PATCH 1/2] Remove unused function parameters from gen_pc_load and 
rename the function
[PATCH 2/2] Remove unused function parameter from cpu_restore_state

Cheers,
Stefan W.