Patchwork explicitly initialize tcg_cpu_thread

login
register
mail settings
Submitter Jun Koi
Date Nov. 1, 2011, 3:35 a.m.
Message ID <CA+g7VZ3NkvnH7LJLP7nh2ojhXztSU--nE8GFpLTQ-B9ZsfikMw@mail.gmail.com>
Download mbox | patch
Permalink /patch/123014/
State New
Headers show

Comments

Jun Koi - Nov. 1, 2011, 3:35 a.m.
This patch explicitly initializes tcg_cpu_thread to NULL in cpus.c
(One code patch in qemu_tcg_init_vcpu() relies on the value of
tcg_cpu_thread to create env->thread and so on )

Signed-off-by: Jun Koi <junkoi2004@gmail.com>
Pavel Borzenkov - Nov. 1, 2011, 7:06 a.m.
On Tue, Nov 1, 2011 at 7:35 AM, Jun Koi <junkoi2004@gmail.com> wrote:
> This patch explicitly initializes tcg_cpu_thread to NULL in cpus.c
> (One code patch in qemu_tcg_init_vcpu() relies on the value of
> tcg_cpu_thread to create env->thread and so on )
>
> Signed-off-by: Jun Koi <junkoi2004@gmail.com>

You don't need to explicitly initialize objects with static storage
duration. They are initialized to NULL/0 implicitly.
This is guaranteed by the C standard.
Jun Koi - Nov. 1, 2011, 8:33 a.m.
On Tue, Nov 1, 2011 at 3:06 PM, Pavel Borzenkov
<pavel.borzenkov@gmail.com> wrote:
> On Tue, Nov 1, 2011 at 7:35 AM, Jun Koi <junkoi2004@gmail.com> wrote:
>> This patch explicitly initializes tcg_cpu_thread to NULL in cpus.c
>> (One code patch in qemu_tcg_init_vcpu() relies on the value of
>> tcg_cpu_thread to create env->thread and so on )
>>
>> Signed-off-by: Jun Koi <junkoi2004@gmail.com>
>
> You don't need to explicitly initialize objects with static storage
> duration. They are initialized to NULL/0 implicitly.
> This is guaranteed by the C standard.

that is good to know, but i think that is better safe than sorry. what
if we compile Qemu with a compiler that doesnt follow the standard?

also, i remember that we always initialize static vars? or am i wrong?

thanks,
Jun




>> diff --git a/cpus.c b/cpus.c
>> index f768683..47feb58 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -606,7 +606,7 @@ static bool iothread_requesting_mutex;
>>
>>  static QemuThread io_thread;
>>
>> -static QemuThread *tcg_cpu_thread;
>> +static QemuThread *tcg_cpu_thread = NULL;
>>  static QemuCond *tcg_halt_cond;
>>
>>  /* cpu creation */
>>
>>
>
Pavel Borzenkov - Nov. 1, 2011, 9:11 a.m.
On Tue, Nov 1, 2011 at 12:33 PM, Jun Koi <junkoi2004@gmail.com> wrote:
> On Tue, Nov 1, 2011 at 3:06 PM, Pavel Borzenkov
> <pavel.borzenkov@gmail.com> wrote:
>> On Tue, Nov 1, 2011 at 7:35 AM, Jun Koi <junkoi2004@gmail.com> wrote:
>>> This patch explicitly initializes tcg_cpu_thread to NULL in cpus.c
>>> (One code patch in qemu_tcg_init_vcpu() relies on the value of
>>> tcg_cpu_thread to create env->thread and so on )
>>>
>>> Signed-off-by: Jun Koi <junkoi2004@gmail.com>
>>
>> You don't need to explicitly initialize objects with static storage
>> duration. They are initialized to NULL/0 implicitly.
>> This is guaranteed by the C standard.
>
> that is good to know, but i think that is better safe than sorry. what
> if we compile Qemu with a compiler that doesnt follow the standard?
>
> also, i remember that we always initialize static vars? or am i wrong?

No, we don't. checkpatch.pl has a check to ensure that static
variables are not explicitly initialized to NULL. Try to check your
patch with this script. It will throw an error:

{{{
ERROR: do not initialise statics to 0 or NULL
#80: FILE: cpus.c:609:
+static QemuThread *tcg_cpu_thread = NULL;

total: 1 errors, 0 warnings, 8 lines checked
}}}

Patch

diff --git a/cpus.c b/cpus.c
index f768683..47feb58 100644
--- a/cpus.c
+++ b/cpus.c
@@ -606,7 +606,7 @@  static bool iothread_requesting_mutex;

 static QemuThread io_thread;

-static QemuThread *tcg_cpu_thread;
+static QemuThread *tcg_cpu_thread = NULL;
 static QemuCond *tcg_halt_cond;

 /* cpu creation */