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

Yoni Bettan posted 1 patch 7 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171218124943.28529-1-ybettan@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
There is a newer version of this series
include/qemu-common.h |  5 -----
linux-user/main.c     |  2 +-
ui/cocoa.m            |  5 ++---
vl.c                  | 15 +--------------
4 files changed, 4 insertions(+), 23 deletions(-)
[Qemu-devel] [PATCH V2] vl.c && linux-user/main.c : removed **envp from main() arguments
Posted by Yoni Bettan 7 years, 12 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

        * 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>
---
 include/qemu-common.h |  5 -----
 linux-user/main.c     |  2 +-
 ui/cocoa.m            |  5 ++---
 vl.c                  | 15 +--------------
 4 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 05319b9ddc..e1bc8690c9 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -27,11 +27,6 @@
     "See <https://qemu.org/contribute/report-a-bug> for how to report bugs.\n" \
     "More information on the QEMU project at <https://qemu.org>."
 
-/* main function, renamed */
-#if defined(CONFIG_COCOA)
-int qemu_main(int argc, char **argv, char **envp);
-#endif
-
 void qemu_get_timedate(struct tm *tm, int offset);
 int qemu_timedate_diff(struct tm *tm);
 
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..851a2dcd06 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 = 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 main(gArgc, gArgv);
             }
         }
     }
diff --git a/vl.c b/vl.c
index fc8bd9372f..d862ec292d 100644
--- a/vl.c
+++ b/vl.c
@@ -35,22 +35,9 @@
 #ifdef CONFIG_SDL
 #if defined(__APPLE__) || defined(main)
 #include <SDL.h>
-int qemu_main(int argc, char **argv, char **envp);
-int main(int argc, char **argv)
-{
-    return qemu_main(argc, argv, NULL);
-}
-#undef main
-#define main qemu_main
 #endif
 #endif /* CONFIG_SDL */
 
-#ifdef CONFIG_COCOA
-#undef main
-#define main qemu_main
-#endif /* CONFIG_COCOA */
-
-
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "hw/hw.h"
@@ -3044,7 +3031,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 V2] vl.c && linux-user/main.c : removed **envp from main() arguments
Posted by Peter Maydell 7 years, 12 months ago
On 18 December 2017 at 12:49, Yoni Bettan <ybettan@redhat.com> wrote:
>          * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
>            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

> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 330ccebf90..851a2dcd06 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 = 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 main(gArgc, gArgv);
>              }
>          }
>      }

Did you test this on OSX? I'm pretty sure it will break,
because now the main() function in ui/cocoa.m will
recursively call itself, rather than calling the
function in vl.c. (Either that or it just won't
link at all because there are two main() functions.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH V2] vl.c && linux-user/main.c : removed **envp from main() arguments
Posted by Yoni Bettan 7 years, 12 months ago

On 12/18/2017 02:54 PM, Peter Maydell wrote:
> On 18 December 2017 at 12:49, Yoni Bettan <ybettan@redhat.com> wrote:
>>           * it was added on 2008 902b3d5c392bb6f48ef340ad8ecc3311705d2800
>>             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
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 330ccebf90..851a2dcd06 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 = 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 main(gArgc, gArgv);
>>               }
>>           }
>>       }
> Did you test this on OSX? I'm pretty sure it will break,
> because now the main() function in ui/cocoa.m will
> recursively call itself, rather than calling the
> function in vl.c. (Either that or it just won't
> link at all because there are two main() functions.)

I didn't checked it on OSX.
I was actually convinced it is Ok but when  I rethink it
I understand that on Mac OSX the third argument is required (char **envp)
as it seems to use it here:  "return qemu_main(gArgc, gArgv, 
*_NSGetEnviron());"
so the patch *idea* will literally break ui/coca.m.

Do you agree?

Thanks,
Yoni
>
> thanks
> -- PMM


Re: [Qemu-devel] [PATCH V2] vl.c && linux-user/main.c : removed **envp from main() arguments
Posted by Peter Maydell 7 years, 12 months ago
On 18 December 2017 at 13:13, Yoni Bettan <ybettan@redhat.com> wrote:
> On 12/18/2017 02:54 PM, Peter Maydell wrote:
>> Did you test this on OSX? I'm pretty sure it will break,
>> because now the main() function in ui/cocoa.m will
>> recursively call itself, rather than calling the
>> function in vl.c. (Either that or it just won't
>> link at all because there are two main() functions.)
>
>
> I didn't checked it on OSX.
> I was actually convinced it is Ok but when  I rethink it
> I understand that on Mac OSX the third argument is required (char **envp)
> as it seems to use it here:  "return qemu_main(gArgc, gArgv,
> *_NSGetEnviron());"
> so the patch *idea* will literally break ui/coca.m.

No, the general idea of removing the 3rd argument from
qemu_main() is fine (you can just not pass the environment
to it from the ui/cocoa.m code). But you can't get rid
of the ifdef stuff that allows cocoa.m to provide its
own main() function and rename the vl.c one to qemu_main.

thanks
-- PMM