diff mbox series

remove qemu-options* from root directory

Message ID 20210517121908.2624991-1-pbonzini@redhat.com
State New
Headers show
Series remove qemu-options* from root directory | expand

Commit Message

Paolo Bonzini May 17, 2021, 12:19 p.m. UTC 
These headers are also included from softmmu/vl.c, so they should be
in include/.  Removing qemu-options-wrapper.h, since elsewhere
we include "template" headers directly and #define the parameters in
the including file, and move qemu-options.h to include/.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.h => include/qemu/qemu-options.h |  9 ++++-
 os-posix.c                                    |  2 +-
 os-win32.c                                    |  1 -
 qemu-options-wrapper.h                        | 40 -------------------
 qemu-options.hx                               |  4 ++
 softmmu/vl.c                                  | 24 ++++++++---
 6 files changed, 31 insertions(+), 49 deletions(-)
 rename qemu-options.h => include/qemu/qemu-options.h (88%)
 delete mode 100644 qemu-options-wrapper.h

Comments

no-reply@patchew.org May 17, 2021, 12:27 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20210517121908.2624991-1-pbonzini@redhat.com/



Hi,

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

Type: series
Message-id: 20210517121908.2624991-1-pbonzini@redhat.com
Subject: [PATCH] remove qemu-options* from root directory

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20210517070851.857841-1-f4bug@amsat.org -> patchew/20210517070851.857841-1-f4bug@amsat.org
 * [new tag]         patchew/20210517121908.2624991-1-pbonzini@redhat.com -> patchew/20210517121908.2624991-1-pbonzini@redhat.com
Switched to a new branch 'test'
fa84df5 remove qemu-options* from root directory

=== OUTPUT BEGIN ===
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
rename from qemu-options.h

ERROR: Macros with complex values should be enclosed in parenthesis
#37: FILE: include/qemu/qemu-options.h:33:
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+    opt_enum,

total: 1 errors, 1 warnings, 79 lines checked

Commit fa84df5703e0 (remove qemu-options* from root directory) 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


The full log is available at
http://patchew.org/logs/20210517121908.2624991-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Markus Armbruster May 18, 2021, 8:54 a.m. UTC | #2 
Paolo Bonzini <pbonzini@redhat.com> writes:

> These headers are also included from softmmu/vl.c, so they should be
> in include/.  Removing qemu-options-wrapper.h, since elsewhere
> we include "template" headers directly and #define the parameters in
> the including file, and move qemu-options.h to include/.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-options.h => include/qemu/qemu-options.h |  9 ++++-
>  os-posix.c                                    |  2 +-
>  os-win32.c                                    |  1 -
>  qemu-options-wrapper.h                        | 40 -------------------
>  qemu-options.hx                               |  4 ++
>  softmmu/vl.c                                  | 24 ++++++++---
>  6 files changed, 31 insertions(+), 49 deletions(-)
>  rename qemu-options.h => include/qemu/qemu-options.h (88%)
>  delete mode 100644 qemu-options-wrapper.h

Much nicer without qemu-options-wrapper.h.

I'd be tempted to rename qemu-options.def while there (what's .def?),
but that's up to you.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Paolo Bonzini May 18, 2021, 10:33 a.m. UTC | #3 
On 18/05/21 10:54, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> These headers are also included from softmmu/vl.c, so they should be
>> in include/.  Removing qemu-options-wrapper.h, since elsewhere
>> we include "template" headers directly and #define the parameters in
>> the including file, and move qemu-options.h to include/.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   qemu-options.h => include/qemu/qemu-options.h |  9 ++++-
>>   os-posix.c                                    |  2 +-
>>   os-win32.c                                    |  1 -
>>   qemu-options-wrapper.h                        | 40 -------------------
>>   qemu-options.hx                               |  4 ++
>>   softmmu/vl.c                                  | 24 ++++++++---
>>   6 files changed, 31 insertions(+), 49 deletions(-)
>>   rename qemu-options.h => include/qemu/qemu-options.h (88%)
>>   delete mode 100644 qemu-options-wrapper.h
> 
> Much nicer without qemu-options-wrapper.h.
> 
> I'd be tempted to rename qemu-options.def while there (what's .def?),
> but that's up to you.
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

I was tempted too, but qemu-options.h is already taken (well, 
qemu/qemu-options.h) and I didn't have any good ideas about the name.

Paolo
Markus Armbruster May 18, 2021, 10:57 a.m. UTC | #4 
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/05/21 10:54, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> These headers are also included from softmmu/vl.c, so they should be
>>> in include/.  Removing qemu-options-wrapper.h, since elsewhere
>>> we include "template" headers directly and #define the parameters in
>>> the including file, and move qemu-options.h to include/.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>   qemu-options.h => include/qemu/qemu-options.h |  9 ++++-
>>>   os-posix.c                                    |  2 +-
>>>   os-win32.c                                    |  1 -
>>>   qemu-options-wrapper.h                        | 40 -------------------
>>>   qemu-options.hx                               |  4 ++
>>>   softmmu/vl.c                                  | 24 ++++++++---
>>>   6 files changed, 31 insertions(+), 49 deletions(-)
>>>   rename qemu-options.h => include/qemu/qemu-options.h (88%)
>>>   delete mode 100644 qemu-options-wrapper.h
>> 
>> Much nicer without qemu-options-wrapper.h.
>> 
>> I'd be tempted to rename qemu-options.def while there (what's .def?),
>> but that's up to you.
>> 
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> I was tempted too, but qemu-options.h is already taken (well, 
> qemu/qemu-options.h) and I didn't have any good ideas about the name.

qemu-options.inc?
Paolo Bonzini May 18, 2021, 12:30 p.m. UTC | #5 
On 18/05/21 12:57, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 18/05/21 10:54, Markus Armbruster wrote:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> These headers are also included from softmmu/vl.c, so they should be
>>>> in include/.  Removing qemu-options-wrapper.h, since elsewhere
>>>> we include "template" headers directly and #define the parameters in
>>>> the including file, and move qemu-options.h to include/.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>    qemu-options.h => include/qemu/qemu-options.h |  9 ++++-
>>>>    os-posix.c                                    |  2 +-
>>>>    os-win32.c                                    |  1 -
>>>>    qemu-options-wrapper.h                        | 40 -------------------
>>>>    qemu-options.hx                               |  4 ++
>>>>    softmmu/vl.c                                  | 24 ++++++++---
>>>>    6 files changed, 31 insertions(+), 49 deletions(-)
>>>>    rename qemu-options.h => include/qemu/qemu-options.h (88%)
>>>>    delete mode 100644 qemu-options-wrapper.h
>>>
>>> Much nicer without qemu-options-wrapper.h.
>>>
>>> I'd be tempted to rename qemu-options.def while there (what's .def?),
>>> but that's up to you.
>>>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>
>> I was tempted too, but qemu-options.h is already taken (well,
>> qemu/qemu-options.h) and I didn't have any good ideas about the name.
> 
> qemu-options.inc?

Or .h.inc, in which case it might make sense to rename other files 
generated from HXTOOL.

Paolo
diff mbox series

Patch

diff --git a/qemu-options.h b/include/qemu/qemu-options.h
similarity index 88%
rename from qemu-options.h
rename to include/qemu/qemu-options.h
index b4ee63cd60..4a62c83c45 100644
--- a/qemu-options.h
+++ b/include/qemu/qemu-options.h
@@ -29,8 +29,13 @@ 
 #define QEMU_OPTIONS_H
 
 enum {
-#define QEMU_OPTIONS_GENERATE_ENUM
-#include "qemu-options-wrapper.h"
+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+    opt_enum,
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
+#include "qemu-options.def"
 };
 
 #endif
diff --git a/os-posix.c b/os-posix.c
index a6846f51c1..ae6c9f2a5e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -32,7 +32,7 @@ 
 #include "qemu-common.h"
 /* Needed early for CONFIG_BSD etc. */
 #include "net/slirp.h"
-#include "qemu-options.h"
+#include "qemu/qemu-options.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "sysemu/runstate.h"
diff --git a/os-win32.c b/os-win32.c
index fd1137bab1..e31c921983 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -27,7 +27,6 @@ 
 #include <windows.h>
 #include <mmsystem.h>
 #include "qemu-common.h"
-#include "qemu-options.h"
 #include "sysemu/runstate.h"
 
 static BOOL WINAPI qemu_ctrl_handler(DWORD type)
diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
deleted file mode 100644
index 6f548e3922..0000000000
--- a/qemu-options-wrapper.h
+++ /dev/null
@@ -1,40 +0,0 @@ 
-
-#if defined(QEMU_OPTIONS_GENERATE_ENUM)
-
-#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
-    opt_enum,
-#define DEFHEADING(text)
-#define ARCHHEADING(text, arch_mask)
-
-#elif defined(QEMU_OPTIONS_GENERATE_HELP)
-
-#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
-    if ((arch_mask) & arch_type)                               \
-        fputs(opt_help, stdout);
-
-#define ARCHHEADING(text, arch_mask) \
-    if ((arch_mask) & arch_type)    \
-        puts(stringify(text));
-
-#define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
-
-#elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
-
-#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
-    { option, opt_arg, opt_enum, arch_mask },
-#define DEFHEADING(text)
-#define ARCHHEADING(text, arch_mask)
-
-#else
-#error "qemu-options-wrapper.h included with no option defined"
-#endif
-
-#include "qemu-options.def"
-
-#undef DEF
-#undef DEFHEADING
-#undef ARCHHEADING
-
-#undef QEMU_OPTIONS_GENERATE_ENUM
-#undef QEMU_OPTIONS_GENERATE_HELP
-#undef QEMU_OPTIONS_GENERATE_OPTIONS
diff --git a/qemu-options.hx b/qemu-options.hx
index ecdb064409..8116f79818 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -5276,3 +5276,7 @@  ERST
 
 
 HXCOMM This is the last statement. Insert new options before this line!
+
+#undef DEF
+#undef DEFHEADING
+#undef ARCHHEADING
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 11ac3750d8..ac0ff6e160 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -88,7 +88,7 @@ 
 #include "qapi/qobject-input-visitor.h"
 #include "qemu/option.h"
 #include "qemu/config-file.h"
-#include "qemu-options.h"
+#include "qemu/qemu-options.h"
 #include "qemu/main-loop.h"
 #ifdef CONFIG_VIRTFS
 #include "fsdev/qemu-fsdev.h"
@@ -854,8 +854,17 @@  static void help(int exitcode)
            "'disk_image' is a raw hard disk image for IDE hard disk 0\n\n",
             error_get_progname());
 
-#define QEMU_OPTIONS_GENERATE_HELP
-#include "qemu-options-wrapper.h"
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
+    if ((arch_mask) & arch_type)                               \
+        fputs(opt_help, stdout);
+
+#define ARCHHEADING(text, arch_mask) \
+    if ((arch_mask) & arch_type)    \
+        puts(stringify(text));
+
+#define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
+
+#include "qemu-options.def"
 
     printf("\nDuring emulation, the following keys are useful:\n"
            "ctrl-alt-f      toggle full screen\n"
@@ -880,8 +889,13 @@  typedef struct QEMUOption {
 
 static const QEMUOption qemu_options[] = {
     { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
-#define QEMU_OPTIONS_GENERATE_OPTIONS
-#include "qemu-options-wrapper.h"
+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+    { option, opt_arg, opt_enum, arch_mask },
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
+#include "qemu-options.def"
     { NULL },
 };