2:01 a.m.
*** BLURB HERE ***
BLURB
Michal Privoznik (4):
examples: Properly include getopt.h
vmwarevertest: drop VIR_FROM_THIS definition
lib: Fix c99 style comments
cpu: Avoid c99 style of assembler
examples/admin/logging.c | 14 +++++++-------
examples/openauth/openauth.c | 2 +-
src/cpu/cpu_ppc64.c | 4 ++--
src/cpu/cpu_x86.c | 44 +++++++++++++++++++++----------------------
src/lxc/lxc_controller.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/rpc/virnetservermdns.c | 2 +-
src/security/security_stack.c | 8 ++++----
src/uml/uml_conf.c | 2 +-
src/util/virhostcpu.c | 4 ++--
src/util/virhostmem.c | 4 ++--
src/vbox/vbox_snapshot_conf.c | 2 +-
tests/eventtest.c | 2 +-
tests/virhostcpumock.c | 2 +-
tests/vmwarevertest.c | 2 --
15 files changed, 47 insertions(+), 49 deletions(-)
--
2.10.2
2:01 a.m.
New subject: [libvirt] [PATCH 1/4] examples: Properly include getopt.h
In the admin/logging.c example, the getopt() function is used but
without proper #include. It relies on unistd.h to subsequently
include getopt.h. This is not necessarily always the case.
Also, opterr is not needed and actually not used anywhere else in
our code.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/admin/logging.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/examples/admin/logging.c b/examples/admin/logging.c
index 648d7a6..de258c2 100644
--- a/examples/admin/logging.c
+++ b/examples/admin/logging.c
@@ -1,11 +1,12 @@
-#include<stdio.h>
-#include<stdlib.h>
-#include<stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
#include "config.h"
-#include<unistd.h>
-#include<libvirt/libvirt-admin.h>
-#include<libvirt/virterror.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <libvirt/libvirt-admin.h>
+#include <libvirt/virterror.h>
static void printHelp(const char *argv0)
{
@@ -31,7 +32,6 @@ int main(int argc, char **argv)
const char *set_filters = NULL;
ret = c = -1;
- opterr = 0;
while ((c = getopt(argc, argv, ":hpo:f:")) > 0) {
switch (c) {
--
2.10.2
2:33 a.m.
New subject: [libvirt] [PATCH 1/4] examples: Properly include getopt.h
On Thu, Apr 27, 2017 at 09:01:57 +0200, Michal Privoznik wrote:
In the admin/logging.c example, the getopt() function is used but
without proper #include. It relies on unistd.h to subsequently
include getopt.h. This is not necessarily always the case.
Also, opterr is not needed and actually not used anywhere else in
our code.
man 3 opterr:
* If the caller has set the global variable opterr to zero, then
* getopt() does not print an error message. The caller
can determine that there was an error by testing whether the
function return value is '?'. (By default, opterr has a
nonzero value.)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/admin/logging.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/examples/admin/logging.c b/examples/admin/logging.c
index 648d7a6..de258c2 100644
--- a/examples/admin/logging.c
+++ b/examples/admin/logging.c
@@ -1,11 +1,12 @@
-#include<stdio.h>
-#include<stdlib.h>
-#include<stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
#include "config.h"
-#include<unistd.h>
-#include<libvirt/libvirt-admin.h>
-#include<libvirt/virterror.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <libvirt/libvirt-admin.h>
+#include <libvirt/virterror.h>
static void printHelp(const char *argv0)
{
@@ -31,7 +32,6 @@ int main(int argc, char **argv)
const char *set_filters = NULL;
ret = c = -1;
- opterr = 0;
NACK to this.
2:01 a.m.
New subject: [libvirt] [PATCH 2/4] vmwarevertest: drop VIR_FROM_THIS definition
Firstly, this definition is in a c99 comment, secondly it is not
needed as VIR_FROM_THIS is defined from vmware/vmware_conf.h.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/vmwarevertest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/vmwarevertest.c b/tests/vmwarevertest.c
index 3dbe08b..e0e36fe 100644
--- a/tests/vmwarevertest.c
+++ b/tests/vmwarevertest.c
@@ -27,8 +27,6 @@
# include "vmware/vmware_conf.h"
-//# define VIR_FROM_THIS VIR_FROM_NONE
-
struct testInfo {
const char *vmware_type;
const char *name;
--
2.10.2
2:36 a.m.
New subject: [libvirt] [PATCH 2/4] vmwarevertest: drop VIR_FROM_THIS definition
On Thu, Apr 27, 2017 at 09:01:58 +0200, Michal Privoznik wrote:
Firstly, this definition is in a c99 comment, secondly it is not
needed as VIR_FROM_THIS is defined from vmware/vmware_conf.h.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/vmwarevertest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/vmwarevertest.c b/tests/vmwarevertest.c
index 3dbe08b..e0e36fe 100644
--- a/tests/vmwarevertest.c
+++ b/tests/vmwarevertest.c
@@ -27,8 +27,6 @@
# include "vmware/vmware_conf.h"
-//# define VIR_FROM_THIS VIR_FROM_NONE
-
This test uses the virAsprintf macro which uses the VIR_FROM_THIS macro.
I think the test file should set it explicitly rather than depend on the
one defined by including the header.
3:19 a.m.
New subject: [libvirt] [PATCH 2/4] vmwarevertest: drop VIR_FROM_THIS definition
On Thu, Apr 27, 2017 at 09:36:19AM +0200, Peter Krempa wrote:
On Thu, Apr 27, 2017 at 09:01:58 +0200, Michal Privoznik wrote:
> Firstly, this definition is in a c99 comment, secondly it is not
> needed as VIR_FROM_THIS is defined from vmware/vmware_conf.h.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> tests/vmwarevertest.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tests/vmwarevertest.c b/tests/vmwarevertest.c
> index 3dbe08b..e0e36fe 100644
> --- a/tests/vmwarevertest.c
> +++ b/tests/vmwarevertest.c
> @@ -27,8 +27,6 @@
>
> # include "vmware/vmware_conf.h"
>
> -//# define VIR_FROM_THIS VIR_FROM_NONE
> -
This test uses the virAsprintf macro which uses the VIR_FROM_THIS macro.
I think the test file should set it explicitly rather than depend on the
one defined by including the header.
Almost all our code puts the VIR_FROM_THIS define in the .c file, so IMHO
we should fix vmware_conf.h to match
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:01 a.m.
New subject: [libvirt] [PATCH 3/4] lib: Fix c99 style comments
We prefer c89 style of comments.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/openauth/openauth.c | 2 +-
src/lxc/lxc_controller.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/rpc/virnetservermdns.c | 2 +-
src/security/security_stack.c | 8 ++++----
src/uml/uml_conf.c | 2 +-
src/util/virhostcpu.c | 4 ++--
src/util/virhostmem.c | 4 ++--
src/vbox/vbox_snapshot_conf.c | 2 +-
tests/eventtest.c | 2 +-
tests/virhostcpumock.c | 2 +-
11 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/examples/openauth/openauth.c b/examples/openauth/openauth.c
index 7a8254b..eef46d5 100644
--- a/examples/openauth/openauth.c
+++ b/examples/openauth/openauth.c
@@ -216,7 +216,7 @@ static virConnectAuth auth = {
credTypes,
sizeof(credTypes) / sizeof(int),
authCallback,
- NULL, // cbdata will be initialized in main
+ NULL, /* cbdata will be initialized in main */
};
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 88ba8aa..1b9d215 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1800,7 +1800,7 @@ virLXCControllerSetupHostdevCaps(virDomainDefPtr vmDef,
securityDriver);
case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET:
- return 0; // case is handled in virLXCControllerMoveInterfaces
+ return 0; /* case is handled in virLXCControllerMoveInterfaces */
default:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 1242bd6..77250ea 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -1145,7 +1145,7 @@ doRemoteOpen(virConnectPtr conn,
if (!(priv->closeCallback = virNewConnectCloseCallbackData()))
goto failed;
- // ref on behalf of netclient
+ /* ref on behalf of netclient */
virObjectRef(priv->closeCallback);
virNetClientSetCloseCallback(priv->client,
remoteClientCloseFunc,
diff --git a/src/rpc/virnetservermdns.c b/src/rpc/virnetservermdns.c
index 7f12a29..8d7df24 100644
--- a/src/rpc/virnetservermdns.c
+++ b/src/rpc/virnetservermdns.c
@@ -128,7 +128,7 @@ static void virNetServerMDNSGroupCallback(AvahiEntryGroup *g
ATTRIBUTE_UNUSED,
avahi_strerror(avahi_client_errno(group->mdns->client)));
/* Some kind of failure happened while we were registering our services */
- //avahi_simple_poll_quit(simple_poll);
+ /* avahi_simple_poll_quit(simple_poll); */
break;
case AVAHI_ENTRY_GROUP_UNCOMMITED:
diff --git a/src/security/security_stack.c b/src/security/security_stack.c
index b02ee18..9a1a7b3 100644
--- a/src/security/security_stack.c
+++ b/src/security/security_stack.c
@@ -210,7 +210,7 @@ virSecurityStackGenLabel(virSecurityManagerPtr mgr,
if (virSecurityManagerGenLabel(virSecurityStackGetPrimary(mgr), vm) < 0)
rc = -1;
-// TODO
+/* TODO */
#if 0
/* We don't allow secondary drivers to generate labels.
* This may have to change in the future, but requires
@@ -235,7 +235,7 @@ virSecurityStackReleaseLabel(virSecurityManagerPtr mgr,
if (virSecurityManagerReleaseLabel(virSecurityStackGetPrimary(mgr), vm) < 0)
rc = -1;
-// TODO
+/* TODO */
#if 0
/* XXX See note in GenLabel */
if (virSecurityManagerReleaseLabel(priv->secondary, vm) < 0)
@@ -255,7 +255,7 @@ virSecurityStackReserveLabel(virSecurityManagerPtr mgr,
if (virSecurityManagerReserveLabel(virSecurityStackGetPrimary(mgr), vm, pid) < 0)
rc = -1;
-// TODO
+/* TODO */
#if 0
/* XXX See note in GenLabel */
if (virSecurityManagerReserveLabel(priv->secondary, vm, pid) < 0)
@@ -460,7 +460,7 @@ virSecurityStackGetProcessLabel(virSecurityManagerPtr mgr,
{
int rc = 0;
-// TODO
+/* TODO */
#if 0
if (virSecurityManagerGetProcessLabel(priv->secondary, vm, pid, seclabel) < 0)
rc = -1;
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index bdef783..d223af0 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -399,7 +399,7 @@ virCommandPtr umlBuildCommandLine(virConnectPtr conn,
virCommandAddEnvPassCommon(cmd);
- //virCommandAddArgPair(cmd, "con0", "fd:0,fd:1");
+ /* virCommandAddArgPair(cmd, "con0", "fd:0,fd:1"); */
virCommandAddArgFormat(cmd, "mem=%lluK", vm->def->mem.cur_balloon);
virCommandAddArgPair(cmd, "umid", vm->def->name);
virCommandAddArgPair(cmd, "uml_dir", driver->monitorDir);
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index a660e3f..8397307 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -795,8 +795,8 @@ virHostCPUGetStatsLinux(FILE *procstat,
if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
if (sscanf(buf,
- "%*s %llu %llu %llu %llu %llu" // user ~ iowait
- "%llu %llu %llu %llu %llu", // irq ~ guest_nice
+ "%*s %llu %llu %llu %llu %llu" /* user ~ iowait */
+ "%llu %llu %llu %llu %llu", /* irq ~ guest_nice */
&usr, &ni, &sys, &idle, &iowait,
&irq, &softirq, &steal, &guest, &guest_nice)
< 4) {
continue;
diff --git a/src/util/virhostmem.c b/src/util/virhostmem.c
index 2ac90a0..a9ba278 100644
--- a/src/util/virhostmem.c
+++ b/src/util/virhostmem.c
@@ -158,8 +158,8 @@ virHostMemGetStatsLinux(FILE *meminfo,
char meminfo_hdr[VIR_NODE_MEMORY_STATS_FIELD_LENGTH];
unsigned long val;
struct field_conv {
- const char *meminfo_hdr; // meminfo header
- const char *field; // MemoryStats field name
+ const char *meminfo_hdr; /* meminfo header */
+ const char *field; /* MemoryStats field name */
} field_conv[] = {
{"MemTotal:", VIR_NODE_MEMORY_STATS_TOTAL},
{"MemFree:", VIR_NODE_MEMORY_STATS_FREE},
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 8bf7ef3..c3b2855 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -958,7 +958,7 @@ virVBoxSnapshotConfRemoveHardDisk(virVBoxSnapshotConfMediaRegistryPtr
mediaRegis
goto cleanup;
}
if (hardDisk->parent == NULL) {
- //it means that the hard disk is in 'root'
+ /* it means that the hard disk is in 'root' */
for (i = 0; i < mediaRegistry->ndisks; i++) {
if (hardDisk == mediaRegistry->disks[i])
break;
diff --git a/tests/eventtest.c b/tests/eventtest.c
index ea47fb0..0c1ec71 100644
--- a/tests/eventtest.c
+++ b/tests/eventtest.c
@@ -523,7 +523,7 @@ mymain(void)
if (finishJob("Write duplicate", 1, -1) != EXIT_SUCCESS)
return EXIT_FAILURE;
- //pthread_kill(eventThread, SIGTERM);
+ /* pthread_kill(eventThread, SIGTERM); */
return EXIT_SUCCESS;
}
diff --git a/tests/virhostcpumock.c b/tests/virhostcpumock.c
index 00d10f1..48147b2 100644
--- a/tests/virhostcpumock.c
+++ b/tests/virhostcpumock.c
@@ -26,7 +26,7 @@ virHostCPUGetThreadsPerSubcore(virArch arch)
{
int threads_per_subcore = 0;
- // Emulate SMT=8 on POWER hardware
+ /* Emulate SMT=8 on POWER hardware */
if (ARCH_IS_PPC64(arch))
threads_per_subcore = 8;
--
2.10.2
2:29 a.m.
New subject: [libvirt] [PATCH 3/4] lib: Fix c99 style comments
On Thu, Apr 27, 2017 at 09:01:59 +0200, Michal Privoznik wrote:
We prefer c89 style of comments.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/openauth/openauth.c | 2 +-
src/lxc/lxc_controller.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/rpc/virnetservermdns.c | 2 +-
src/security/security_stack.c | 8 ++++----
src/uml/uml_conf.c | 2 +-
src/util/virhostcpu.c | 4 ++--
src/util/virhostmem.c | 4 ++--
src/vbox/vbox_snapshot_conf.c | 2 +-
tests/eventtest.c | 2 +-
tests/virhostcpumock.c | 2 +-
11 files changed, 16 insertions(+), 16 deletions(-)
ACK
3:19 a.m.
New subject: [libvirt] [PATCH 3/4] lib: Fix c99 style comments
On Thu, Apr 27, 2017 at 09:01:59AM +0200, Michal Privoznik wrote:
We prefer c89 style of comments.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
examples/openauth/openauth.c | 2 +-
src/lxc/lxc_controller.c | 2 +-
src/remote/remote_driver.c | 2 +-
src/rpc/virnetservermdns.c | 2 +-
src/security/security_stack.c | 8 ++++----
src/uml/uml_conf.c | 2 +-
src/util/virhostcpu.c | 4 ++--
src/util/virhostmem.c | 4 ++--
src/vbox/vbox_snapshot_conf.c | 2 +-
tests/eventtest.c | 2 +-
tests/virhostcpumock.c | 2 +-
11 files changed, 16 insertions(+), 16 deletions(-)
How about a syntax-check rule to enforce this ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:09 a.m.
New subject: [libvirt] [PATCH 3/4] lib: Fix c99 style comments
On 04/27/2017 10:19 AM, Daniel P. Berrange wrote:
On Thu, Apr 27, 2017 at 09:01:59AM +0200, Michal Privoznik wrote:
> We prefer c89 style of comments.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> examples/openauth/openauth.c | 2 +-
> src/lxc/lxc_controller.c | 2 +-
> src/remote/remote_driver.c | 2 +-
> src/rpc/virnetservermdns.c | 2 +-
> src/security/security_stack.c | 8 ++++----
> src/uml/uml_conf.c | 2 +-
> src/util/virhostcpu.c | 4 ++--
> src/util/virhostmem.c | 4 ++--
> src/vbox/vbox_snapshot_conf.c | 2 +-
> tests/eventtest.c | 2 +-
> tests/virhostcpumock.c | 2 +-
> 11 files changed, 16 insertions(+), 16 deletions(-)
How about a syntax-check rule to enforce this ?
It's going to be not trivial. Consider:
/* My awesome comment, that contains
* http://an.url.com
*/
It's not easy to know that line 2 of the comment is already in comment.
We can't just forbid all '//' strings.
Michal
4:22 a.m.
New subject: [libvirt] [PATCH 3/4] lib: Fix c99 style comments
On Thu, Apr 27, 2017 at 11:09:29AM +0200, Michal Privoznik wrote:
On 04/27/2017 10:19 AM, Daniel P. Berrange wrote:
> On Thu, Apr 27, 2017 at 09:01:59AM +0200, Michal Privoznik wrote:
>> We prefer c89 style of comments.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> examples/openauth/openauth.c | 2 +-
>> src/lxc/lxc_controller.c | 2 +-
>> src/remote/remote_driver.c | 2 +-
>> src/rpc/virnetservermdns.c | 2 +-
>> src/security/security_stack.c | 8 ++++----
>> src/uml/uml_conf.c | 2 +-
>> src/util/virhostcpu.c | 4 ++--
>> src/util/virhostmem.c | 4 ++--
>> src/vbox/vbox_snapshot_conf.c | 2 +-
>> tests/eventtest.c | 2 +-
>> tests/virhostcpumock.c | 2 +-
>> 11 files changed, 16 insertions(+), 16 deletions(-)
>
> How about a syntax-check rule to enforce this ?
It's going to be not trivial. Consider:
/* My awesome comment, that contains
* http://an.url.com
*/
It's not easy to know that line 2 of the comment is already in comment.
We can't just forbid all '//' strings.
Our build-aux/bracket-spacing.pl script knows how to strip out /* ... */
It also strips // out, so we could just adapt it to whine when it sees
// comments, rather than try to do it using the cfg.mk rules.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:02 a.m.
New subject: [libvirt] [PATCH 4/4] cpu: Avoid c99 style of assembler
In c89 - the standard we claim to support - there is no asm()
rather than __asm__(). Switch from the former to the latter.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/cpu/cpu_ppc64.c | 4 ++--
src/cpu/cpu_x86.c | 44 ++++++++++++++++++++++----------------------
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index f64592b..5d65929 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -732,8 +732,8 @@ virCPUppc64GetHost(virCPUDefPtr cpu,
data->len = 1;
#if defined(__powerpc__) || defined(__powerpc64__)
- asm("mfpvr %0"
- : "=r" (data->pvr[0].value));
+ __asm__("mfpvr %0"
+ : "=r" (data->pvr[0].value));
#endif
data->pvr[0].mask = 0xfffffffful;
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 53359ff..c971aa3 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2085,32 +2085,32 @@ static inline void
cpuidCall(virCPUx86CPUID *cpuid)
{
# if __x86_64__
- asm("xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */
- "xor %%edx, %%edx;" /* functions may use them as additional arguments
*/
- "cpuid;"
- : "=a" (cpuid->eax),
- "=b" (cpuid->ebx),
- "=c" (cpuid->ecx),
- "=d" (cpuid->edx)
- : "a" (cpuid->eax_in),
- "c" (cpuid->ecx_in));
+ __asm__("xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */
+ "xor %%edx, %%edx;" /* functions may use them as additional
arguments */
+ "cpuid;"
+ : "=a" (cpuid->eax),
+ "=b" (cpuid->ebx),
+ "=c" (cpuid->ecx),
+ "=d" (cpuid->edx)
+ : "a" (cpuid->eax_in),
+ "c" (cpuid->ecx_in));
# else
/* we need to avoid direct use of ebx for CPUID output as it is used
* for global offset table on i386 with -fPIC
*/
- asm("push %%ebx;"
- "xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */
- "xor %%edx, %%edx;" /* functions may use them as additional arguments
*/
- "cpuid;"
- "mov %%ebx, %1;"
- "pop %%ebx;"
- : "=a" (cpuid->eax),
- "=r" (cpuid->ebx),
- "=c" (cpuid->ecx),
- "=d" (cpuid->edx)
- : "a" (cpuid->eax_in),
- "c" (cpuid->ecx_in)
- : "cc");
+ __asm__("push %%ebx;"
+ "xor %%ebx, %%ebx;" /* clear the other registers as some cpuid */
+ "xor %%edx, %%edx;" /* functions may use them as additional
arguments */
+ "cpuid;"
+ "mov %%ebx, %1;"
+ "pop %%ebx;"
+ : "=a" (cpuid->eax),
+ "=r" (cpuid->ebx),
+ "=c" (cpuid->ecx),
+ "=d" (cpuid->edx)
+ : "a" (cpuid->eax_in),
+ "c" (cpuid->ecx_in)
+ : "cc");
# endif
}
--
2.10.2
2:24 a.m.
New subject: [libvirt] [PATCH 4/4] cpu: Avoid c99 style of assembler
On Thu, Apr 27, 2017 at 09:02:00 +0200, Michal Privoznik wrote:
In c89 - the standard we claim to support - there is no asm()
I'm not quite sure about the truth of this statement. Especially after
commits like:
0f6c46c3d virsh.c: Switch to C99 initialization of vshCmdOptDef
879d409e9 Convert all driver struct intializers to C99 style
and quite a lot more ...
rather than __asm__(). Switch from the former to the latter.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/cpu/cpu_ppc64.c | 4 ++--
src/cpu/cpu_x86.c | 44 ++++++++++++++++++++++----------------------
2 files changed, 24 insertions(+), 24 deletions(-)
3:22 a.m.
New subject: [libvirt] [PATCH 4/4] cpu: Avoid c99 style of assembler
On Thu, Apr 27, 2017 at 09:24:54AM +0200, Peter Krempa wrote:
On Thu, Apr 27, 2017 at 09:02:00 +0200, Michal Privoznik wrote:
> In c89 - the standard we claim to support - there is no asm()
I'm not quite sure about the truth of this statement. Especially after
commits like:
0f6c46c3d virsh.c: Switch to C99 initialization of vshCmdOptDef
879d409e9 Convert all driver struct intializers to C99 style
and quite a lot more ...
Yep the C99 struct member initializers are a very valuable feature.
I'd say, we aim for c89, plus selected c99 features.
We explicitly try to avoid some other c99 features though, such variables
declared in the middle of functions.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:30 a.m.
New subject: [libvirt] [PATCH 4/4] cpu: Avoid c99 style of assembler
On Thu, Apr 27, 2017 at 09:22:04AM +0100, Daniel P. Berrange wrote:
On Thu, Apr 27, 2017 at 09:24:54AM +0200, Peter Krempa wrote:
> On Thu, Apr 27, 2017 at 09:02:00 +0200, Michal Privoznik wrote:
> > In c89 - the standard we claim to support - there is no asm()
>
> I'm not quite sure about the truth of this statement. Especially after
> commits like:
>
> 0f6c46c3d virsh.c: Switch to C99 initialization of vshCmdOptDef
> 879d409e9 Convert all driver struct intializers to C99 style
>
> and quite a lot more ...
Yep the C99 struct member initializers are a very valuable feature.
I'd say, we aim for c89, plus selected c99 features.
We explicitly try to avoid some other c99 features though, such variables
declared in the middle of functions.
Looking at the gcc/clang docs...
By default GCC applies a C standard called gnu89/gnu90, which is basically
c89/c90, but with a tonne of GCC extensions, many of which are present in
the newer c99. Actually using c89 is impractical since it lacks even
basic things like "inline" and var-args for macros.
CLang aims to be compatible with GCC extensions, but in fact defaults to
the even newer gnu11 standard, which is C11 + GCC extensions
It looks like the only GCC extensions we rely on from gnu89, are all
present in c99 - at least build succeeds with both std=gnu89 and std=c99
So, it is probably accurate to say we're compliant with c99, and if a
compiler doesn't support c99, it must support the GNU extensions to
c89.
In practice we only care about GCC & CLang.
So I guess the question from thsi patch series is mostly, which c99
features do we wish to explicitly avoid using.
Variable declarations in the middle of blocks is one we avoid, just
because it harms readability and understanding of code when goto is
just to jump over a variable declaration.
I see no problem using c99 asm statements, unless we're using a mix
of different asm syntaxes across the code base, in which case we
should standardize on one.
I tend to think it is worth avoiding C++ comments, just for sake of
having a consistent style across the code base.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2766
days inactive
2766
days old
14 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Michal Privoznik -
Peter Krempa