Patchwork [03/10] linux-user: Move target_to_host_errno_table[] setup out of ioctl loop

login
register
mail settings
Submitter Peter Maydell
Date Aug. 14, 2012, 9:40 a.m.
Message ID <1344937236-611-4-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/177207/
State New
Headers show

Comments

Peter Maydell - Aug. 14, 2012, 9:40 a.m.
The code to initialise the target_to_host_errno_table[] array was
accidentally inside the loop through checking and initialising all
the supported ioctls. This was harmless but meant that we reinitialised the
array several hundred times on startup.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/syscall.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
Blue Swirl - Aug. 14, 2012, 6:27 p.m.
On Tue, Aug 14, 2012 at 9:40 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The code to initialise the target_to_host_errno_table[] array was
> accidentally inside the loop through checking and initialising all
> the supported ioctls. This was harmless but meant that we reinitialised the
> array several hundred times on startup.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/syscall.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 3ba3ef5..1af68d2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -4606,6 +4606,11 @@ void syscall_init(void)
>  #undef STRUCT
>  #undef STRUCT_SPECIAL
>
> +    /* Build target_to_host_errno_table[] table from
> +     * host_to_target_errno_table[]. */
> +    for (i=0; i < ERRNO_TABLE_SIZE; i++)

Please add spaces around '=' and braces.

> +        target_to_host_errno_table[host_to_target_errno_table[i]] = i;
> +
>      /* we patch the ioctl size if necessary. We rely on the fact that
>         no ioctl has all the bits at '1' in the size field */
>      ie = ioctl_entries;
> @@ -4625,11 +4630,6 @@ void syscall_init(void)
>                  (size << TARGET_IOC_SIZESHIFT);
>          }
>
> -        /* Build target_to_host_errno_table[] table from
> -         * host_to_target_errno_table[]. */
> -        for (i=0; i < ERRNO_TABLE_SIZE; i++)
> -                target_to_host_errno_table[host_to_target_errno_table[i]] = i;
> -
>          /* automatic consistency check if same arch */
>  #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \
>      (defined(__x86_64__) && defined(TARGET_X86_64))
> --
> 1.7.9.5
>
Peter Maydell - Aug. 14, 2012, 6:38 p.m.
On 14 August 2012 19:27, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Aug 14, 2012 at 9:40 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> The code to initialise the target_to_host_errno_table[] array was
>> accidentally inside the loop through checking and initialising all
>> the supported ioctls. This was harmless but meant that we reinitialised the
>> array several hundred times on startup.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  linux-user/syscall.c |   10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 3ba3ef5..1af68d2 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -4606,6 +4606,11 @@ void syscall_init(void)
>>  #undef STRUCT
>>  #undef STRUCT_SPECIAL
>>
>> +    /* Build target_to_host_errno_table[] table from
>> +     * host_to_target_errno_table[]. */
>> +    for (i=0; i < ERRNO_TABLE_SIZE; i++)
>
> Please add spaces around '=' and braces.

I agree that might be nice but (a) this is just code motion (b) it's
a bit impolite to make review comments at the final pull request
stage rather than when the patches were posted on the list for
review some weeks ago.

-- PMM
Blue Swirl - Aug. 14, 2012, 6:58 p.m.
On Tue, Aug 14, 2012 at 6:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 14 August 2012 19:27, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Aug 14, 2012 at 9:40 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> The code to initialise the target_to_host_errno_table[] array was
>>> accidentally inside the loop through checking and initialising all
>>> the supported ioctls. This was harmless but meant that we reinitialised the
>>> array several hundred times on startup.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  linux-user/syscall.c |   10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 3ba3ef5..1af68d2 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -4606,6 +4606,11 @@ void syscall_init(void)
>>>  #undef STRUCT
>>>  #undef STRUCT_SPECIAL
>>>
>>> +    /* Build target_to_host_errno_table[] table from
>>> +     * host_to_target_errno_table[]. */
>>> +    for (i=0; i < ERRNO_TABLE_SIZE; i++)
>>
>> Please add spaces around '=' and braces.
>
> I agree that might be nice but (a) this is just code motion

Since pure CODING_STYLE patches are not appreciated, the only way to
fix style issues is to always handle them when code is touched.

> (b) it's
> a bit impolite to make review comments at the final pull request
> stage rather than when the patches were posted on the list for
> review some weeks ago.

I agree, but I don't review linux-user patches so often.

>
> -- PMM

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 3ba3ef5..1af68d2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4606,6 +4606,11 @@  void syscall_init(void)
 #undef STRUCT
 #undef STRUCT_SPECIAL
 
+    /* Build target_to_host_errno_table[] table from
+     * host_to_target_errno_table[]. */
+    for (i=0; i < ERRNO_TABLE_SIZE; i++)
+        target_to_host_errno_table[host_to_target_errno_table[i]] = i;
+
     /* we patch the ioctl size if necessary. We rely on the fact that
        no ioctl has all the bits at '1' in the size field */
     ie = ioctl_entries;
@@ -4625,11 +4630,6 @@  void syscall_init(void)
                 (size << TARGET_IOC_SIZESHIFT);
         }
 
-        /* Build target_to_host_errno_table[] table from
-         * host_to_target_errno_table[]. */
-        for (i=0; i < ERRNO_TABLE_SIZE; i++)
-                target_to_host_errno_table[host_to_target_errno_table[i]] = i;
-
         /* automatic consistency check if same arch */
 #if (defined(__i386__) && defined(TARGET_I386) && defined(TARGET_ABI32)) || \
     (defined(__x86_64__) && defined(TARGET_X86_64))