[Qemu-devel] Re: [PATCH 16/28] migration: use global variable directly

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/23/2011 04:46 PM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>    
>> On 02/23/2011 03:47 PM, Juan Quintela wrote:
>>      
>>> We are setting a pointer to a local variable in the previous line, just use
>>> the global variable directly.  We remove the ->file test because it is already
>>> done inside qemu_file_set_rate_limit() function.
>>>
>>>        
>> I think this is bad form generally speaking.  Globals are not
>> something to be embraced but rather to be isolated as much as humanly
>> possible.
>>      
> current_migration is a global variable.
>
> And just doing:
>
> s = current_migration;
>
> foo(s);
>
> helps nothing.

It's still bad form IMHO.  You should always use local variables to 
reference global variables unless you're explicitly setting a global 
variable.

Regards,

Anthony Liguori

>   We are not going to happen more than one migration at
> the same time anytime soon.  Notice that everything that is outside of
> migration.c  already receives a pointer to it, i.e. not use the global
> one.  So, I claim this is a very special case, not the normal case about
> global variables.
>
> Later, Juan.
>
>    
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>> ---
>>>    migration.c |    6 ++----
>>>    1 files changed, 2 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/migration.c b/migration.c
>>> index caf5044..21f5a9a 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -457,7 +457,6 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>    int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>    {
>>>        int64_t d;
>>> -    MigrationState *s;
>>>
>>>        d = qdict_get_int(qdict, "value");
>>>        if (d<   0) {
>>> @@ -465,9 +464,8 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, QObject **ret_data)
>>>        }
>>>        max_throttle = d;
>>>
>>> -    s = current_migration;
>>> -    if (s&&   s->file) {
>>> -        qemu_file_set_rate_limit(s->file, max_throttle);
>>> +    if (current_migration) {
>>> +        qemu_file_set_rate_limit(current_migration->file, max_throttle);
>>>        }
>>>
>>>        return 0;
>>>
>>>