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

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

[Yoni Bettan]

it was added on 2008 902b3d5c39 when introduced cache-utils.[ch]

since then cache-utils.[ch] were removed but **envp was left
behind.

By the way "to be portable it is best to write main to take two
arguments, and use the value of environ" according to
https://www.gnu.org/software/libc/manual/html_node/Program-\
      Arguments.html#Program-Arguments

Signed-off-by: Yoni Bettan <ybettan@redhat.com>
---

V2 -> V3:
    
    since ui/cocoa.c rename main() is using qemu_main() ,because it uses it 
    inside its main() function, the function qemu_main() can't be removed
  

V1 -> V2:

    removed the ui/cocoa.c renaming of main() function since i thought that we 
    no longer need qemu_main()


 include/qemu-common.h | 2 +-
 linux-user/main.c     | 2 +-
 ui/cocoa.m            | 5 ++---
 vl.c                  | 7 +++----
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 05319b9ddc..8a5b7ec5ec 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -29,7 +29,7 @@
 
 /* main function, renamed */
 #if defined(CONFIG_COCOA)
-int qemu_main(int argc, char **argv, char **envp);
+int qemu_main(int argc, char **argv);
 #endif
 
 void qemu_get_timedate(struct tm *tm, int offset);
diff --git a/linux-user/main.c b/linux-user/main.c
index 6286661bd3..fe81d410da 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -4237,7 +4237,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/ui/cocoa.m b/ui/cocoa.m
index 330ccebf90..4d18153659 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -25,7 +25,6 @@
 #include "qemu/osdep.h"
 
 #import <Cocoa/Cocoa.h>
-#include <crt_externs.h>
 
 #include "qemu-common.h"
 #include "ui/console.h"
@@ -1050,7 +1049,7 @@ QemuCocoaView *cocoaView;
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
 
     int status;
-    status = qemu_main(argc, argv, *_NSGetEnviron());
+    status = qemu_main(argc, argv);
     exit(status);
 }
 
@@ -1391,7 +1390,7 @@ int main (int argc, const char * argv[]) {
                 !strcmp(opt, "-curses") ||
                 !strcmp(opt, "-display") ||
                 !strcmp(opt, "-qtest")) {
-                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
+                return qemu_main(gArgc, gArgv);
             }
         }
     }
diff --git a/vl.c b/vl.c
index fc8bd9372f..ea17cc34f6 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
@@ -50,7 +50,6 @@ int main(int argc, char **argv)
 #define main qemu_main
 #endif /* CONFIG_COCOA */
 
-
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "hw/hw.h"
@@ -3044,7 +3043,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.14.3

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

[Yoni Bettan]

On 12/19/2017 12:11 PM, Yoni Bettan wrote:
> it was added on 2008 902b3d5c39 when introduced cache-utils.[ch]
>
> since then cache-utils.[ch] were removed but **envp was left
> behind.
>
> By the way "to be portable it is best to write main to take two
> arguments, and use the value of environ" according to
> https://www.gnu.org/software/libc/manual/html_node/Program-\
>        Arguments.html#Program-Arguments
>
> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
> ---
>
> V2 -> V3:
>      
>      since ui/cocoa.c rename main() is using qemu_main() ,because it uses it
>      inside its main() function, the function qemu_main() can't be removed
>    
>
> V1 -> V2:
>
>      removed the ui/cocoa.c renaming of main() function since i thought that we
>      no longer need qemu_main()
>
>
>   include/qemu-common.h | 2 +-
>   linux-user/main.c     | 2 +-
>   ui/cocoa.m            | 5 ++---
>   vl.c                  | 7 +++----
>   4 files changed, 7 insertions(+), 9 deletions(-)

Notes:
1. I didn't checked the patch on Mac OS due to some difficulties to run
     Mac OS gust as host (make check passed successfully)
2. checkpatch.pl seems to not work well when touching extern variables
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 05319b9ddc..8a5b7ec5ec 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -29,7 +29,7 @@
>   
>   /* main function, renamed */
>   #if defined(CONFIG_COCOA)
> -int qemu_main(int argc, char **argv, char **envp);
> +int qemu_main(int argc, char **argv);
>   #endif
>   
>   void qemu_get_timedate(struct tm *tm, int offset);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 6286661bd3..fe81d410da 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4237,7 +4237,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/ui/cocoa.m b/ui/cocoa.m
> index 330ccebf90..4d18153659 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -25,7 +25,6 @@
>   #include "qemu/osdep.h"
>   
>   #import <Cocoa/Cocoa.h>
> -#include <crt_externs.h>
>   
>   #include "qemu-common.h"
>   #include "ui/console.h"
> @@ -1050,7 +1049,7 @@ QemuCocoaView *cocoaView;
>       COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>   
>       int status;
> -    status = qemu_main(argc, argv, *_NSGetEnviron());
> +    status = qemu_main(argc, argv);
>       exit(status);
>   }
>   
> @@ -1391,7 +1390,7 @@ int main (int argc, const char * argv[]) {
>                   !strcmp(opt, "-curses") ||
>                   !strcmp(opt, "-display") ||
>                   !strcmp(opt, "-qtest")) {
> -                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> +                return qemu_main(gArgc, gArgv);
>               }
>           }
>       }
> diff --git a/vl.c b/vl.c
> index fc8bd9372f..ea17cc34f6 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
> @@ -50,7 +50,6 @@ int main(int argc, char **argv)
>   #define main qemu_main
>   #endif /* CONFIG_COCOA */
>   
> -
>   #include "qemu/error-report.h"
>   #include "qemu/sockets.h"
>   #include "hw/hw.h"
> @@ -3044,7 +3043,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;

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

[Laurent Vivier]

Le 19/12/2017 à 11:11, Yoni Bettan a écrit :
> it was added on 2008 902b3d5c39 when introduced cache-utils.[ch]
> 
> since then cache-utils.[ch] were removed but **envp was left
> behind.
> 
> By the way "to be portable it is best to write main to take two
> arguments, and use the value of environ" according to
> https://www.gnu.org/software/libc/manual/html_node/Program-\
>       Arguments.html#Program-Arguments
> 
> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
> ---
> 
> V2 -> V3:
>     
>     since ui/cocoa.c rename main() is using qemu_main() ,because it uses it 
>     inside its main() function, the function qemu_main() can't be removed
>   
> 
> V1 -> V2:
> 
>     removed the ui/cocoa.c renaming of main() function since i thought that we 
>     no longer need qemu_main()
> 
> 
>  include/qemu-common.h | 2 +-
>  linux-user/main.c     | 2 +-
>  ui/cocoa.m            | 5 ++---
>  vl.c                  | 7 +++----
>  4 files changed, 7 insertions(+), 9 deletions(-)
> 
...
> diff --git a/vl.c b/vl.c
> index fc8bd9372f..ea17cc34f6 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'm really sorry, but I really think we can remove this part.

This does not break the call of qemu_main from COCOA because we keep:

     48 #ifdef CONFIG_COCOA
     49 #undef main
     50 #define main qemu_main
     51 #endif /* CONFIG_COCOA */

Thanks,
Laurent

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

[no-reply]

Hi,

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

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

=== 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
 t [tag update]            patchew/20171219082518.19724-1-lma@suse.com -> patchew/20171219082518.19724-1-lma@suse.com
Switched to a new branch 'test'
354502e84f 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
#85: FILE: vl.c:38:
+int qemu_main(int argc, char **argv);

total: 1 errors, 0 warnings, 66 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 V3] vl.c && linux-user/main.c : removed **envp from main() arguments

[Yoni Bettan]

On 12/19/2017 12:21 PM, Laurent Vivier wrote:
> Le 19/12/2017 à 11:11, Yoni Bettan a écrit :
>> it was added on 2008 902b3d5c39 when introduced cache-utils.[ch]
>>
>> since then cache-utils.[ch] were removed but **envp was left
>> behind.
>>
>> By the way "to be portable it is best to write main to take two
>> arguments, and use the value of environ" according to
>> https://www.gnu.org/software/libc/manual/html_node/Program-\
>>        Arguments.html#Program-Arguments
>>
>> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
>> ---
>>
>> V2 -> V3:
>>      
>>      since ui/cocoa.c rename main() is using qemu_main() ,because it uses it
>>      inside its main() function, the function qemu_main() can't be removed
>>    
>>
>> V1 -> V2:
>>
>>      removed the ui/cocoa.c renaming of main() function since i thought that we
>>      no longer need qemu_main()
>>
>>
>>   include/qemu-common.h | 2 +-
>>   linux-user/main.c     | 2 +-
>>   ui/cocoa.m            | 5 ++---
>>   vl.c                  | 7 +++----
>>   4 files changed, 7 insertions(+), 9 deletions(-)
>>
> ...
>> diff --git a/vl.c b/vl.c
>> index fc8bd9372f..ea17cc34f6 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'm really sorry, but I really think we can remove this part.

Can you please show what exact lines you think can be removed?
If you meant that the entire part can be removed then how can
we rename main to qemu_main (in the CONFIG_COCA part) if qemu_main is 
not defined?

Thanks,
Yoni
>
> This does not break the call of qemu_main from COCOA because we keep:
>
>       48 #ifdef CONFIG_COCOA
>       49 #undef main
>       50 #define main qemu_main
>       51 #endif /* CONFIG_COCOA */
>
> Thanks,
> Laurent

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

[Laurent Vivier]

Le 19/12/2017 à 14:05, Yoni Bettan a écrit :
> 
> 
> On 12/19/2017 12:21 PM, Laurent Vivier wrote:
>> Le 19/12/2017 à 11:11, Yoni Bettan a écrit :
>>> it was added on 2008 902b3d5c39 when introduced cache-utils.[ch]
>>>
>>> since then cache-utils.[ch] were removed but **envp was left
>>> behind.
>>>
>>> By the way "to be portable it is best to write main to take two
>>> arguments, and use the value of environ" according to
>>> https://www.gnu.org/software/libc/manual/html_node/Program-\
>>>        Arguments.html#Program-Arguments
>>>
>>> Signed-off-by: Yoni Bettan <ybettan@redhat.com>
>>> ---
>>>
>>> V2 -> V3:
>>>           since ui/cocoa.c rename main() is using qemu_main()
>>> ,because it uses it
>>>      inside its main() function, the function qemu_main() can't be
>>> removed
>>>   
>>> V1 -> V2:
>>>
>>>      removed the ui/cocoa.c renaming of main() function since i
>>> thought that we
>>>      no longer need qemu_main()
>>>
>>>
>>>   include/qemu-common.h | 2 +-
>>>   linux-user/main.c     | 2 +-
>>>   ui/cocoa.m            | 5 ++---
>>>   vl.c                  | 7 +++----
>>>   4 files changed, 7 insertions(+), 9 deletions(-)
>>>
>> ...
>>> diff --git a/vl.c b/vl.c
>>> index fc8bd9372f..ea17cc34f6 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'm really sorry, but I really think we can remove this part.
> 
> Can you please show what exact lines you think can be removed?
> If you meant that the entire part can be removed then how can
> we rename main to qemu_main (in the CONFIG_COCA part) if qemu_main is
> not defined?

Yes, the entire part can be removed.

The following lines will declare the qemu_main for COCOA:
>>
>>       48 #ifdef CONFIG_COCOA
>>       49 #undef main
>>       50 #define main qemu_main
>>       51 #endif /* CONFIG_COCOA */

The C preprocessor will replace:

int main(int argc, char **argv, char **envp)
{
...

by

int qemu_main(int argc, char **argv, char **envp)
{
...

To check, you can try:

-----8<---------------- main.c
#define main qemu_main

int main(void)
{
    return 0;
}
-----8<---------------- main.c

then: cc -E main.c

# 1 "main.c"
# 1 "<built-in>"
# 1 "<command-line>"
# 31 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 32 "<command-line>" 2
# 1 "main.c"


int qemu_main(void)
{
    return 0;
}


Thanks,
Laurent