Re: [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c

[Darren Kenny <darren.kenny@oracle.com>]

On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>A program might rely on functions implemented in vl.c, but implement its
>own main(). By placing main into a separate source file, there are no
>complaints about duplicate main()s when linking against vl.o. For
>example, the virtual-device fuzzer uses a main() provided by libfuzzer,
>and needs to perform some initialization before running the softmmu
>initialization. Now, main simply calls three vl.c functions which
>handle the guest initialization, main loop and cleanup.
>
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>---
> Makefile                |  1 +
> Makefile.objs           |  2 ++
> include/sysemu/sysemu.h |  4 ++++
> main.c                  | 52 +++++++++++++++++++++++++++++++++++++++++
> vl.c                    | 36 +++++++---------------------
> 5 files changed, 68 insertions(+), 27 deletions(-)
> create mode 100644 main.c
>
>diff --git a/Makefile b/Makefile
>index 0e994a275d..d2b2ecd3c4 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -474,6 +474,7 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y)
> $(SOFTMMU_ALL_RULES): $(io-obj-y)
> $(SOFTMMU_ALL_RULES): config-all-devices.mak
> $(SOFTMMU_ALL_RULES): $(edk2-decompressed)
>+$(SOFTMMU_ALL_RULES): $(softmmu-main-y)
>
> .PHONY: $(TARGET_DIRS_RULES)
> # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that
>diff --git a/Makefile.objs b/Makefile.objs
>index 11ba1a36bd..9ff9b0c6f9 100644
>--- a/Makefile.objs
>+++ b/Makefile.objs
>@@ -86,6 +86,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o
> # qapi
>
> common-obj-y += qapi/
>+
>+softmmu-main-y = main.o
> endif
>
> #######################################################################
>diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>index 44f18eb739..03f9838b81 100644
>--- a/include/sysemu/sysemu.h
>+++ b/include/sysemu/sysemu.h
>@@ -114,6 +114,10 @@ QemuOpts *qemu_get_machine_opts(void);
>
> bool defaults_enabled(void);
>
>+void main_loop(void);
>+void qemu_init(int argc, char **argv, char **envp);
>+void qemu_cleanup(void);
>+
> extern QemuOptsList qemu_legacy_drive_opts;
> extern QemuOptsList qemu_common_drive_opts;
> extern QemuOptsList qemu_drive_opts;
>diff --git a/main.c b/main.c
>new file mode 100644
>index 0000000000..ecd6389424
>--- /dev/null
>+++ b/main.c
>@@ -0,0 +1,52 @@
>+/*
>+ * QEMU System Emulator
>+ *
>+ * Copyright (c) 2003-2008 Fabrice Bellard
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a copy
>+ * of this software and associated documentation files (the "Software"), to deal
>+ * in the Software without restriction, including without limitation the rights
>+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>+ * copies of the Software, and to permit persons to whom the Software is
>+ * furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be included in
>+ * all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>+ * THE SOFTWARE.
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "sysemu/sysemu.h"
>+
>+#ifdef CONFIG_SDL
>+#if defined(__APPLE__) || defined(main)
>+#include <SDL.h>
>+int main(int argc, char **argv)
>+{
>+    return qemu_main(argc, argv, NULL);
>+}
>+#undef main
>+#define main qemu_main

This /looks/ wrong, you're defining a function main(), and then
immediately #undef and #define main again.

Maybe this could be written differently, or add a comment here as to
why you need to do this.

>+#endif
>+#endif /* CONFIG_SDL */
>+
>+#ifdef CONFIG_COCOA
>+#undef main
>+#define main qemu_main
>+#endif /* CONFIG_COCOA */

I don't really know the combinations that might exist, but it looks
like if CONFIG_SDL is not defined, then we're redefining main() to be
qemi_main() - so what main() function will actually be used here?

Thanks,

Darren.

>+
>+int main(int argc, char **argv, char **envp)
>+{
>+    qemu_init(argc, argv, envp);
>+    main_loop();
>+    qemu_cleanup();
>+
>+    return 0;
>+}
>diff --git a/vl.c b/vl.c
>index c389d24b2c..472f09e12a 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -36,25 +36,6 @@
> #include "sysemu/seccomp.h"
> #include "sysemu/tcg.h"
>
>-#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 "sysemu/accel.h"
>@@ -1797,7 +1778,7 @@ static bool main_loop_should_exit(void)
>     return false;
> }
>
>-static void main_loop(void)
>+void main_loop(void)
> {
> #ifdef CONFIG_PROFILER
>     int64_t ti;
>@@ -2824,7 +2805,7 @@ static void user_register_global_props(void)
>                       global_init_func, NULL, NULL);
> }
>
>-int main(int argc, char **argv, char **envp)
>+void qemu_init(int argc, char **argv, char **envp)
> {
>     int i;
>     int snapshot, linux_boot;
>@@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp)
>             case QEMU_OPTION_watchdog:
>                 if (watchdog) {
>                     error_report("only one watchdog option may be given");
>-                    return 1;
>+                    exit(1);
>                 }
>                 watchdog = optarg;
>                 break;
>@@ -4440,7 +4421,7 @@ int main(int argc, char **argv, char **envp)
>     if (vmstate_dump_file) {
>         /* dump and exit */
>         dump_vmstate_json_to_file(vmstate_dump_file);
>-        return 0;
>+        exit(0);
>     }
>
>     if (incoming) {
>@@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp)
>     accel_setup_post(current_machine);
>     os_setup_post();
>
>-    main_loop();
>+    return;
>+}
>
>+void qemu_cleanup(void)
>+{
>     gdbserver_cleanup();
>
>     /*
>@@ -4495,6 +4479,4 @@ int main(int argc, char **argv, char **envp)
>     qemu_chr_cleanup();
>     user_creatable_cleanup();
>     /* TODO: unref root container, check all devices are ok */
>-
>-    return 0;
> }
>-- 
>2.23.0
>
>