[v1] vl.c && linux-user/main.c : removed **envp from main() arguments

[Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Yoni Bettan 6 years, 5 months ago

        * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
          when introduced cache-utils.[ch]
        * since then cache-utils.[ch] were removed but **envp was left
          behind

Signed-off-by: Yoni Bettan <ybettan@redhat.com>
---
 linux-user/main.c | 2 +-
 vl.c              | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index aa02f25b85..ca5628c1ca 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4233,7 +4233,7 @@ static int parse_args(int argc, char **argv)
     return optind;
 }
 
-int main(int argc, char **argv, char **envp)
+int main(int argc, char **argv)
 {
     struct target_pt_regs regs1, *regs = &regs1;
     struct image_info info1, *info = &info1;
diff --git a/vl.c b/vl.c
index 1ad1c04637..9667756ccc 100644
--- a/vl.c
+++ b/vl.c
@@ -35,10 +35,10 @@
 #ifdef CONFIG_SDL
 #if defined(__APPLE__) || defined(main)
 #include <SDL.h>
-int qemu_main(int argc, char **argv, char **envp);
+int qemu_main(int argc, char **argv);
 int main(int argc, char **argv)
 {
-    return qemu_main(argc, argv, NULL);
+    return qemu_main(argc, argv);
 }
 #undef main
 #define main qemu_main
@@ -3088,7 +3088,7 @@ static void register_global_properties(MachineState *ms)
     user_register_global_props();
 }
 
-int main(int argc, char **argv, char **envp)
+int main(int argc, char **argv)
 {
     int i;
     int snapshot, linux_boot;
-- 
2.13.6

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by no-reply@patchew.org 6 years, 5 months ago

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments
Type: series
Message-id: 20171121064106.13721-1-ybettan@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20171121064106.13721-1-ybettan@redhat.com -> patchew/20171121064106.13721-1-ybettan@redhat.com
Switched to a new branch 'test'
7657b24e5a vl.c && linux-user/main.c : removed **envp from main() arguments

=== OUTPUT BEGIN ===
Checking PATCH 1/1: vl.c && linux-user/main.c : removed **envp from main() arguments...
ERROR: externs should be avoided in .c files
#37: FILE: vl.c:38:
+int qemu_main(int argc, char **argv);

total: 1 errors, 0 warnings, 28 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Laurent Vivier 6 years, 5 months ago

Le 21/11/2017 à 07:41, Yoni Bettan a écrit :
>         * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
>           when introduced cache-utils.[ch]
>         * since then cache-utils.[ch] were removed but **envp was left
>           behind
> 
> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
> ---
>  linux-user/main.c | 2 +-
>  vl.c              | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index aa02f25b85..ca5628c1ca 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4233,7 +4233,7 @@ static int parse_args(int argc, char **argv)
>      return optind;
>  }
>  
> -int main(int argc, char **argv, char **envp)
> +int main(int argc, char **argv)
>  {
>      struct target_pt_regs regs1, *regs = &regs1;
>      struct image_info info1, *info = &info1;
> diff --git a/vl.c b/vl.c
> index 1ad1c04637..9667756ccc 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -35,10 +35,10 @@
>  #ifdef CONFIG_SDL
>  #if defined(__APPLE__) || defined(main)
>  #include <SDL.h>
> -int qemu_main(int argc, char **argv, char **envp);
> +int qemu_main(int argc, char **argv);
>  int main(int argc, char **argv)
>  {
> -    return qemu_main(argc, argv, NULL);
> +    return qemu_main(argc, argv);
>  }
>  #undef main
>  #define main qemu_main

I think this part can be removed now. As it seems it has been added
because of the envp parameter.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Yoni Bettan 6 years, 4 months ago


On 11/21/2017 09:30 AM, Laurent Vivier wrote:
> Le 21/11/2017 à 07:41, Yoni Bettan a écrit :
>>          * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
>>            when introduced cache-utils.[ch]
>>          * since then cache-utils.[ch] were removed but **envp was left
>>            behind
>>
>> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
>> ---
>>   linux-user/main.c | 2 +-
>>   vl.c              | 6 +++---
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index aa02f25b85..ca5628c1ca 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -4233,7 +4233,7 @@ static int parse_args(int argc, char **argv)
>>       return optind;
>>   }
>>   
>> -int main(int argc, char **argv, char **envp)
>> +int main(int argc, char **argv)
>>   {
>>       struct target_pt_regs regs1, *regs = &regs1;
>>       struct image_info info1, *info = &info1;
>> diff --git a/vl.c b/vl.c
>> index 1ad1c04637..9667756ccc 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -35,10 +35,10 @@
>>   #ifdef CONFIG_SDL
>>   #if defined(__APPLE__) || defined(main)
>>   #include <SDL.h>
>> -int qemu_main(int argc, char **argv, char **envp);
>> +int qemu_main(int argc, char **argv);
>>   int main(int argc, char **argv)
>>   {
>> -    return qemu_main(argc, argv, NULL);
>> +    return qemu_main(argc, argv);
>>   }
>>   #undef main
>>   #define main qemu_main
> I think this part can be removed now. As it seems it has been added
> because of the envp parameter.

Thanks Laurent for your response and sorry for my late response.
I think this part was added for another purpose as described here 
880fec5d086

As i see it the env param was added in 2008 in order to support cache-utils
as shown in 902b3d5c392bb6f48ef340ad8ecc3311705d2800 and in 2009
another main function was added in order to unbreak SDL on Mac-OS X and 
the env param
was added only for consistency I suppose...

What do you think?

Thanks,
Yoni
> Thanks,
> Laurent

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Laurent Vivier 6 years, 4 months ago

Le 18/12/2017 à 07:07, Yoni Bettan a écrit :
> 
> 
> On 11/21/2017 09:30 AM, Laurent Vivier wrote:
>> Le 21/11/2017 à 07:41, Yoni Bettan a écrit :
>>>          * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
>>>            when introduced cache-utils.[ch]
>>>          * since then cache-utils.[ch] were removed but **envp was left
>>>            behind
>>>
>>> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
>>> ---
>>>   linux-user/main.c | 2 +-
>>>   vl.c              | 6 +++---
>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/linux-user/main.c b/linux-user/main.c
>>> index aa02f25b85..ca5628c1ca 100644
>>> --- a/linux-user/main.c
>>> +++ b/linux-user/main.c
>>> @@ -4233,7 +4233,7 @@ static int parse_args(int argc, char **argv)
>>>       return optind;
>>>   }
>>>   -int main(int argc, char **argv, char **envp)
>>> +int main(int argc, char **argv)
>>>   {
>>>       struct target_pt_regs regs1, *regs = &regs1;
>>>       struct image_info info1, *info = &info1;
>>> diff --git a/vl.c b/vl.c
>>> index 1ad1c04637..9667756ccc 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -35,10 +35,10 @@
>>>   #ifdef CONFIG_SDL
>>>   #if defined(__APPLE__) || defined(main)
>>>   #include <SDL.h>
>>> -int qemu_main(int argc, char **argv, char **envp);
>>> +int qemu_main(int argc, char **argv);
>>>   int main(int argc, char **argv)
>>>   {
>>> -    return qemu_main(argc, argv, NULL);
>>> +    return qemu_main(argc, argv);
>>>   }
>>>   #undef main
>>>   #define main qemu_main
>> I think this part can be removed now. As it seems it has been added
>> because of the envp parameter.
> 
> Thanks Laurent for your response and sorry for my late response.
> I think this part was added for another purpose as described here
> 880fec5d086
> 
> As i see it the env param was added in 2008 in order to support cache-utils
> as shown in 902b3d5c392bb6f48ef340ad8ecc3311705d2800 and in 2009
> another main function was added in order to unbreak SDL on Mac-OS X and
> the env param
> was added only for consistency I suppose...
> 
> What do you think?

I think we don't need it anymore as there is no difference betweent
qemu_main() and main().

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Peter Maydell 6 years, 4 months ago

On 18 December 2017 at 07:52, Laurent Vivier <laurent@vivier.eu> wrote:
> I think we don't need it anymore as there is no difference betweent
> qemu_main() and main().

You need to be a bit cautious there to avoid breaking
the OSX code in ui/cocoa.m, which has its own version
of main() which kicks off the GUI code and then eventually
calls qemu_main() later.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] vl.c && linux-user/main.c : removed **envp from main() arguments

Posted by Laurent Vivier 6 years, 4 months ago

Le 18/12/2017 à 13:30, Peter Maydell a écrit :
> On 18 December 2017 at 07:52, Laurent Vivier <laurent@vivier.eu> wrote:
>> I think we don't need it anymore as there is no difference betweent
>> qemu_main() and main().
> 
> You need to be a bit cautious there to avoid breaking
> the OSX code in ui/cocoa.m, which has its own version
> of main() which kicks off the GUI code and then eventually
> calls qemu_main() later.

Yes, you're right. cocoa.m needs qemu_main() because there is already a
main() inside. So we can't remove qemu_main().

Thanks,
Laurent