Clang patches
cover Clang patches
From: Blue Swirl <[email protected]>
With this patch set, I'm able to compile AREG0 free softmmu targets.
Blue Swirl (5):
sparc: fix floppy TC line setup
sparc: fix expression with uninitialized initial value
qapi: avoid reserved word restrict
user: fix accidental AREG0 use
configure: disable a few Clang compiler warnings
configure | 5 ++++- hw/sun4m.c | 18 +++++++++++++----- linux-user/signal.c | 2 +- net/slirp.c | 6 +++--- qapi-schema.json | 4 ++-- user-exec.c | 17 ++++++++++++----- 6 files changed, 35 insertions(+), 17 deletions(-)
Signature · 1 line
1.7.2.5
PATCH 1/5 sparc: fix floppy TC line setup
From: Blue Swirl <[email protected]>
The qemu_irq for Terminal Count (TC) line between FDC and Slavio misc device was created only after use, spotted by Clang compiler. Also, it was not created if the FDC didn't exist.
Rearrange code to fix order. Always create the TC line.
--- hw/sun4m.c | 18 +++++++++++++----- 1 files changed, 13 insertions(+), 5 deletions(-)
hw/sun4m.c +13 -5
@@ -832,6 +832,10 @@ static void cpu_devinit(const char *cpu_model, unsigned int id, env->prom_addr = prom_addr; } +static void dummy_fdc_tc (void *opaque,int irq,int level) +{ +} + static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size, const char *boot_device, const char *kernel_filename,
@@ -942,9 +946,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size, serial_hds[0], serial_hds[1], ESCC_CLOCK, 1); cpu_halt =qemu_allocate_irqs (cpu_halt_signal,NULL ,1 ); -slavio_misc_init (hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base, - slavio_irq[30 ], fdc_tc); -if (hwdef->apc_base) {apc_init (hwdef->apc_base, cpu_halt[0 ]); }
@@ -955,8 +956,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size, fd[0 ] =drive_get (IF_FLOPPY ,0 ,0 );sun4m_fdctrl_init (slavio_irq[22 ], hwdef->fd_base, fd, &fdc_tc); + }else { + fdc_tc = *qemu_allocate_irqs (dummy_fdc_tc,NULL ,1 ); } +slavio_misc_init (hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base, + slavio_irq[30 ], fdc_tc); +if (drive_get_max_bus (IF_SCSI ) >0 ) {fprintf (stderr,"qemu: too many SCSI bus\n" );exit (1 );
@@ -1772,16 +1778,18 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size, slavio_irq[1 ], serial_hds[0 ], serial_hds[1 ], ESCC_CLOCK, 1); -slavio_misc_init (0 , hwdef->aux1_base,0 , slavio_irq[1 ], fdc_tc); -if (hwdef->fd_base != (target_phys_addr_t)-1 ) {/* there is zero or one floppy drive */ memset (fd,0 ,sizeof (fd)); fd[0 ] =drive_get (IF_FLOPPY ,0 ,0 );sun4m_fdctrl_init (slavio_irq[1 ], hwdef->fd_base, fd, &fdc_tc); + }else { + fdc_tc = *qemu_allocate_irqs (dummy_fdc_tc,NULL ,1 ); } +slavio_misc_init (0 , hwdef->aux1_base,0 , slavio_irq[1 ], fdc_tc); +if (drive_get_max_bus (IF_SCSI ) >0 ) {fprintf (stderr,"qemu: too many SCSI bus\n" );exit (1 );
Signature · 1 line
1.7.2.5
PATCH 2/5 sparc: fix expression with uninitialized initial value
In reply to [email protected]From: Blue Swirl <[email protected]>
err was uninitalized, it's not OK to use |=. Spotted by Clang compiler.
Fix by replacing |= by =.
--- linux-user/signal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
linux-user/signal.c +1 -1
@@ -2061,7 +2061,7 @@ restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu) err =__copy_from_user (&env->fpr[0 ], &fpu->si_float_regs[0 ], (sizeof (unsigned long ) *32 ));#endif - err |=__get_user (env->fsr, &fpu->si_fsr); + err =__get_user (env->fsr, &fpu->si_fsr);#if 0 err |=__get_user (current->thread.fpqdepth, &fpu->si_fpqdepth);if (current->thread.fpqdepth !=0 )
Signature · 1 line
1.7.2.5
Re: [Qemu-devel] [PATCH 2/5] sparc: fix expression with uninitialized initial value
Quoted 3 lines · On 30 July 2012 17:04, <[email protected]> wrote:
From: Blue Swirl <[email protected]>
err was uninitalized, it's not OK to use |=. Spotted by Clang
"uninitialized" (feel free to just fix typo on commit).
Quoted 14 lines
compiler.
Fix by replacing |= by =.
Signed-off-by Blue Swirl <[email protected]>--- linux-user/signal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)linux-user/signal.c +0
-0
linux-user/signal.c restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu) · line 2067
Quoted 8 lines
linux-user/signal.c +1
-1@@ -2061,7 +2061,7 @@ restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu) err =__copy_from_user (&env->fpr[0 ], &fpu->si_float_regs[0 ], (sizeof (unsigned long ) *32 ));#endif - err |=__get_user (env->fsr, &fpu->si_fsr); + err =__get_user (env->fsr, &fpu->si_fsr);#if 0 err |=__get_user (current->thread.fpqdepth, &fpu->si_fpqdepth);if (current->thread.fpqdepth !=0 )
This will need changing again if we ever fix the #if 0-d out code in this function, but I guess that will be obvious to whoever does that.
Incidentally, __get_user() can never fail [we catch unreadable memory earlier when wo do the lock_user_struct] so you could also just not do anything with its return value. Some of the other targets rely on this in their signal save/restore code. I think the use of the return value is mostly in code that was copy-and-pasted from the Linux kernel (which does use a __get_user() that can fail).
-- PMM
Re: [Qemu-devel] [PATCH 2/5] sparc: fix expression with uninitialized initial value
Am 30.07.2012 18:13, schrieb Peter Maydell:
Quoted 25 lines · On 30 July 2012 17:04, <[email protected]> wrote:
Quoted 3 lines
From: Blue Swirl <[email protected]>
err was uninitalized, it's not OK to use |=. Spotted by Clang
"uninitialized" (feel free to just fix typo on commit).
Quoted 18 lines
compiler.
Fix by replacing |= by =.
Signed-off-by Blue Swirl <[email protected]>--- linux-user/signal.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)linux-user/signal.c +0
-1@@ -2061,7 +2061,7 @@ restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu) err =__copy_from_user (&env->fpr[0 ], &fpu->si_float_regs[0 ], (sizeof (unsigned long ) *32 ));#endif - err |=__get_user (env->fsr, &fpu->si_fsr);
Quoted 8 lines
linux-user/signal.c restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu) · line 2067
Quoted 4 lines
linux-user/signal.c +1
-0+ err =__get_user (env->fsr, &fpu->si_fsr);#if 0 err |=__get_user (current->thread.fpqdepth, &fpu->si_fpqdepth);if (current->thread.fpqdepth !=0 )This will need changing again if we ever fix the #if 0-d out code in this function, but I guess that will be obvious to whoever does that.
You mean the #endif part? Would an explicit err = 0 make things better?
Andreas
Quoted 11 lines
Incidentally,
__get_user()can never fail [we catch unreadable memory earlier when wo do thelock_user_struct] so you could also just not do anything with its return value. Some of the other targets rely on this in their signal save/restore code. I think the use of the return value is mostly in code that was copy-and-pasted from the Linux kernel (which does use a__get_user()that can fail).Reviewed-by Peter Maydell <[email protected]>-- PMM
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/5] sparc: fix expression with uninitialized initial value
Quoted 6 lines · Andreas Färber
Am 30.07.2012 18:13, schrieb Peter Maydell:
Quoted 3 lines
This will need changing again if we ever fix the #if 0-d out code in this function, but I guess that will be obvious to whoever does that.
You mean the #endif part? Would an explicit err = 0 make things better?
Not really, things would still need changing later. Really the problem is that most of the function is #if 0'd out (and never called from anywhere); it's just unused stub code so anything that shuts the compiler up will do, really.
-- PMM
Re: [Qemu-devel] [PATCH 2/5] sparc: fix expression with uninitialized initial value
Quoted 6 lines · Peter Maydell
Quoted 5 lines · Andreas Färber
Am 30.07.2012 18:13, schrieb Peter Maydell:
Quoted 3 lines
This will need changing again if we ever fix the #if 0-d out code in this function, but I guess that will be obvious to whoever does that.
Quoted 6 lines
Quoted 1 line
You mean the #endif part? Would an explicit err = 0 make things better?
Not really, things would still need changing later. Really the problem is that most of the function is #if 0'd out (and never called from anywhere); it's just unused stub code so anything that shuts the compiler up will do, really.
I'll implement the #if 0'd part.
Quoted 2 lines
-- PMM
Re: [Qemu-devel] [PATCH 2/5] sparc: fix expression with uninitialized initial value
Quoted 7 lines · Blue Swirl
Quoted 4 lines · Peter Maydell
Not really, things would still need changing later. Really the problem is that most of the function is #if 0'd out (and never called from anywhere); it's just unused stub code so anything that shuts the compiler up will do, really.
I'll implement the #if 0'd part.
That would be cool, but just to be clear, I don't in general think we should block small cleanup/fix series on "implement $feature first", so I'm not saying you have to do that now.
-- PMM
PATCH 3/5 qapi: avoid reserved word restrict
In reply to [email protected]From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
--- net/slirp.c | 6 +++--- qapi-schema.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
net/slirp.c +3 -3
@@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const char *name,net_init_slirp_configs (user->hostfwd,SLIRP_CFG_HOSTFWD );net_init_slirp_configs (user->guestfwd,0 ); - ret =net_slirp_init (vlan,"user" , name, user->restrict, vnet, user->host, - user->hostname, user->tftp, user->bootfile, - user->dhcpstart, user->dns, user->smb, + ret =net_slirp_init (vlan,"user" , name, user->restricted, vnet, + user->host, user->hostname, user->tftp, + user->bootfile, user->dhcpstart, user->dns, user->smb, user->smbserver);while (slirp_configs) {
qapi-schema.json +2 -2
@@ -1925,7 +1925,7 @@ # # @hostname: #optional client hostname reported by the builtin DHCP server # -# @restrict: #optional isolate the guest from the host +# @restricted: #optional isolate the guest from the host # # @ip: #optional legacy parameter, use net= instead #
@@ -1956,7 +1956,7 @@ { 'type': 'NetdevUserOptions', 'data': { '*hostname': 'str', - '*restrict': 'bool', + '*restricted':'bool', '*ip': 'str', '*net': 'str', '*host': 'str',
Signature · 1 line
1.7.2.5
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
Paolo
Quoted 46 lines
Signed-off-by Blue Swirl <[email protected]>--- net/slirp.c | 6 +++--- qapi-schema.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)net/slirp.c +3
-3@@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const char *name,net_init_slirp_configs (user->hostfwd,SLIRP_CFG_HOSTFWD );net_init_slirp_configs (user->guestfwd,0 ); - ret =net_slirp_init (vlan,"user" , name, user->restrict, vnet, user->host, - user->hostname, user->tftp, user->bootfile, - user->dhcpstart, user->dns, user->smb, + ret =net_slirp_init (vlan,"user" , name, user->restricted, vnet, + user->host, user->hostname, user->tftp, + user->bootfile, user->dhcpstart, user->dns, user->smb, user->smbserver);while (slirp_configs) {qapi-schema.json +2
-2@@ -1925,7 +1925,7 @@ # # @hostname: #optional client hostname reported by the builtin DHCP server # -# @restrict: #optional isolate the guest from the host +# @restricted: #optional isolate the guest from the host # # @ip: #optional legacy parameter, use net= instead #@@ -1956,7 +1956,7 @@ { 'type': 'NetdevUserOptions', 'data': { '*hostname': 'str', - '*restrict': 'bool', + '*restricted':'bool', '*ip': 'str', '*net': 'str', '*host': 'str',
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 11 lines · On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini <[email protected]> wrote:
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this:
{ 'type': 'NetdevUserOptions',
'data': {
'*hostname': 'str',
'*restrict': 'bool',
...
'*hostfwd': ['String'],
'*guestfwd': ['String'] },'bind': { 'restrict': 'restricted' } }
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 20 lines · Luiz Capitulino
Quoted 11 lines · On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini <[email protected]> wrote:
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this:
{ 'type': 'NetdevUserOptions',
Quoted 8 lines
'data': { '*hostname': 'str', '*restrict': 'bool', ... '*hostfwd': ['String'], '*guestfwd': ['String'] },'bind': { 'restrict': 'restricted' } }
How about prefixing all json-generated field names with for example 'json_'? Should be a simple mechanical change.
In addition to 'restrict', there may also be problems with 'if' (-drive, HMP drive_add) and maybe also 'auto' as value (several command line options, HMP pci_add) in the future.
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 29 lines · Blue Swirl
Quoted 27 lines · Luiz Capitulino
Quoted 11 lines · On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini <[email protected]> wrote:
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this:
{ 'type': 'NetdevUserOptions',
'data': { '*hostname': 'str', '*restrict': 'bool', ... '*hostfwd': ['String'], '*guestfwd': ['String'] },
Quoted 4 lines
Quoted 1 line
'bind': { 'restrict': 'restricted' } }
How about prefixing all json-generated field names with for example '
json_'? Should be a simple mechanical change.
It's a whole lot of churn though, and clobbers the history for most QMP functions. It also seems like a strange thing for clang to complain about...
I think special casing is probably the way to go...
Luiz's bind approach seems reasonable, though "alias" might be the more familiar name.
As an alternative I'll throw out there, the QIDL series introduces the notion of "annotated" fields, which allow us to pass additional information to the code generators (instead of just the typename) regarding how to handle that field internally. So we could do something like:
{ 'type': 'NetdevUserOptions',
'data': {
'*hostname': 'str',
'*restrict': {
'<annotated>': 'true',
'type': 'bool',
'native_name': 'restrict' },
...
'*hostfwd': ['String'],
'*guestfwd': ['String'] } }It's pretty flexible, but I'm hesitant to expose in documented schemas.
Quoted 5 lines
In addition to 'restrict', there may also be problems with 'if' (-drive, HMP
drive_add) and maybe also 'auto' as value (several command line options, HMPpci_add) in the future.
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 30 lines · Michael Roth
Quoted 29 lines · Blue Swirl
Quoted 27 lines · Luiz Capitulino
Quoted 11 lines · On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini <[email protected]> wrote:
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this:
{ 'type': 'NetdevUserOptions',
'data': { '*hostname': 'str', '*restrict': 'bool', ... '*hostfwd': ['String'], '*guestfwd': ['String'] },
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
'bind': { 'restrict': 'restricted' } }
How about prefixing all json-generated field names with for example '
json_'? Should be a simple mechanical change.It's a whole lot of churn though, and clobbers the history for most QMP functions. It also seems like a strange thing for clang to complain about...
Not really, it's just C99:
http://en.wikipedia.org/wiki/Restrict
Prefixing would solve also future problems: 'if', 'auto', maybe 'static' can make sense for network options (as compared to DHCP) one day etc.
Quoted 22 lines
I think special casing is probably the way to go...
Luiz's bind approach seems reasonable, though "alias" might be the more familiar name.
As an alternative I'll throw out there, the QIDL series introduces the notion of "annotated" fields, which allow us to pass additional information to the code generators (instead of just the typename) regarding how to handle that field internally. So we could do something like:
{ 'type': 'NetdevUserOptions',
'data': { '*hostname': 'str', '*restrict': { '<annotated>': 'true', 'type': 'bool', 'native_name': 'restrict' }, ... '*hostfwd': ['String'], '*guestfwd': ['String'] } }
Quoted 1 line
It's pretty flexible, but I'm hesitant to expose in documented schemas.
This could work too.
We could also introduce new names and deprecate old ones, one day we could remove the old versions. This is bit drastic change to be done just because a new user invisible implementation makes command line names clash with C keywords. I'd rather prefix.
Quoted 6 lines
Quoted 6 lines
In addition to 'restrict', there may also be problems with 'if' (-drive, HMP
drive_add) and maybe also 'auto' as value (several command line options, HMPpci_add) in the future.
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 33 lines · Blue Swirl
Quoted 32 lines · Michael Roth
Quoted 31 lines · Blue Swirl
Quoted 28 lines · Luiz Capitulino
Quoted 11 lines · On Tue, 31 Jul 2012 09:28:43 +0200 Paolo Bonzini <[email protected]> wrote:
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
I'm not sure how complicated it would be to implement this, but we could add a 'bind' keyword to the type dict to control mapping between protocol names and generated variable names. Like this:
{ 'type': 'NetdevUserOptions',
'data': { '*hostname': 'str', '*restrict': 'bool', ... '*hostfwd': ['String'], '*guestfwd': ['String'] },'bind': { 'restrict': 'restricted' } }
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
How about prefixing all json-generated field names with for example '
json_'? Should be a simple mechanical change.It's a whole lot of churn though, and clobbers the history for most QMP functions. It also seems like a strange thing for clang to complain about...
Not really, it's just C99:
http://en.wikipedia.org/wiki/Restrict
As a struct field it didn't seem like there was a case where the usage would be ambiguous, but yah, it's still justified to complain.
Quoted 5 lines
Prefixing would solve also future problems: 'if', 'auto', maybe 'static' can make sense for network options (as compared to DHCP) one day etc.
I don't think there's a necessary risk of this happening again. We can catch it in code review, warn against it in documentation, and even go as far as adding a list of reserved keywords that the schema parser/code generators can complain about.
(I'm also somewhat biased because QIDL needs the schema names to align with the actual field names, since the schema in that case is a description of an exiting type that is not generated by QAPI. Although it wouldn't be too hard to add a command-line option to still support that...)
Another option I'll put out there is to have the code generators special-case field_name in [ <reserved keywords> ]: to re-name the fields internally, and document the behavior for developers. This keeps our mistakes from spilling out into the QAPI schemas.
Quoted 38 lines
Quoted 22 lines
I think special casing is probably the way to go...
Luiz's bind approach seems reasonable, though "alias" might be the more familiar name.
As an alternative I'll throw out there, the QIDL series introduces the notion of "annotated" fields, which allow us to pass additional information to the code generators (instead of just the typename) regarding how to handle that field internally. So we could do something like:
{ 'type': 'NetdevUserOptions',
'data': { '*hostname': 'str', '*restrict': { '<annotated>': 'true', 'type': 'bool', 'native_name': 'restrict' }, ... '*hostfwd': ['String'], '*guestfwd': ['String'] } }Quoted 1 line
It's pretty flexible, but I'm hesitant to expose in documented schemas.
This could work too.
We could also introduce new names and deprecate old ones, one day we could remove the old versions. This is bit drastic change to be done just because a new user invisible implementation makes command line names clash with C keywords. I'd rather prefix.
Quoted 6 lines
Quoted 6 lines
In addition to 'restrict', there may also be problems with 'if' (-drive, HMP
drive_add) and maybe also 'auto' as value (several command line options, HMPpci_add) in the future.
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
In reply to Blue SwirlIl 31/07/2012 22:38, Blue Swirl ha scritto:
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines
It's a whole lot of churn though, and clobbers the history for most QMP functions. It also seems like a strange thing for clang to complain about...
Not really, it's just C99:
http://en.wikipedia.org/wiki/RestrictPrefixing would solve also future problems: 'if', 'auto', maybe 'static' can make sense for network options (as compared to DHCP) one day etc.
We could detect C keywords and prefix (or suffix) an underscore automatically.
Paolo
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
Quoted 6 lines · Paolo Bonzini
Il 31/07/2012 22:38, Blue Swirl ha scritto:
Quoted 5 lines
Quoted 2 lines
Quoted 2 lines
It's a whole lot of churn though, and clobbers the history for most QMP functions. It also seems like a strange thing for clang to complain about...
Not really, it's just C99:
http://en.wikipedia.org/wiki/Restrict
Quoted 7 lines
Quoted 4 lines
Prefixing would solve also future problems: 'if', 'auto', maybe 'static' can make sense for network options (as compared to DHCP) one day etc.
We could detect C keywords and prefix (or suffix) an underscore automatically.
I'll send a new patch which avoids all C keywords.
Quoted 2 lines
Paolo
Re: [Qemu-devel] [PATCH 3/5] qapi: avoid reserved word restrict
In reply to Paolo BonziniQuoted 11 lines · Paolo Bonzini
Il 30/07/2012 18:04, [email protected] ha scritto:
Quoted 6 lines
From: Blue Swirl <[email protected]>
Clang compiler complained about use of reserved word 'restrict' in SLIRP and QAPI.
Rename 'restrict' to 'restricted' which also matches other SLIRP code.
Can't do it, this changes the command-line option.
Luiz, Michael, any ideas?
Just change c_var() in scripts/qapi.py to have:
scripts/qapi.py +2 -0
@@ -131,6 +131,8 @@ def camel_case(name): return new_name def c_var(name): + if name in ['restrict']: + return 'q_restrict' return name.replace('-', '_').lstrip("*") def c_fun(name):
Then fixup and build failures (make sure to do a clean first--dependencies seem to be off for qapi).
Regards,
Anthony Liguori
Quoted 49 lines
Paolo
Quoted 46 lines
Signed-off-by Blue Swirl <[email protected]>--- net/slirp.c | 6 +++--- qapi-schema.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)net/slirp.c +3
-3@@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const char *name,net_init_slirp_configs (user->hostfwd,SLIRP_CFG_HOSTFWD );net_init_slirp_configs (user->guestfwd,0 ); - ret =net_slirp_init (vlan,"user" , name, user->restrict, vnet, user->host, - user->hostname, user->tftp, user->bootfile, - user->dhcpstart, user->dns, user->smb, + ret =net_slirp_init (vlan,"user" , name, user->restricted, vnet, + user->host, user->hostname, user->tftp, + user->bootfile, user->dhcpstart, user->dns, user->smb, user->smbserver);while (slirp_configs) {qapi-schema.json +2
-2@@ -1925,7 +1925,7 @@ # # @hostname: #optional client hostname reported by the builtin DHCP server # -# @restrict: #optional isolate the guest from the host +# @restricted: #optional isolate the guest from the host # # @ip: #optional legacy parameter, use net= instead #@@ -1956,7 +1956,7 @@ { 'type': 'NetdevUserOptions', 'data': { '*hostname': 'str', - '*restrict': 'bool', + '*restricted':'bool', '*ip': 'str', '*net': 'str', '*host': 'str',
PATCH 4/5 user: fix accidental AREG0 use
In reply to [email protected]From: Blue Swirl <[email protected]>
Global register AREG0 was always assumed to be usable in user-exec.c, but this is incorrect for several targets.
Fix with #ifdeffery and by using other variables.
--- user-exec.c | 17 ++++++++++++----- 1 files changed, 12 insertions(+), 5 deletions(-)
user-exec.c +12 -5
@@ -18,7 +18,9 @@*/ #include "config.h" #include "cpu.h" +#ifndef CONFIG_TCG_PASS_AREG0 #include "dyngen-exec.h" +#endif #include "disas.h" #include "tcg.h"
@@ -58,9 +60,11 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)struct sigcontext *uc = puc;#endif +#ifndef CONFIG_TCG_PASS_AREG0 env = env1;/* XXX: restore cpu registers saved in host registers */ +#endif if (puc) {/* XXX: use siglongjmp ? */
@@ -74,8 +78,8 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)sigprocmask (SIG_SETMASK , &uc->sc_mask,NULL );#endif } - env->exception_index =-1 ; -longjmp (env->jmp_env,1 ); + env1->exception_index =-1 ; +longjmp (env1->jmp_env,1 ); }/* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -89,9 +93,11 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,TranslationBlock *tb;int ret; +#ifndef CONFIG_TCG_PASS_AREG0 if (cpu_single_env) { env = cpu_single_env;/* XXX: find a correct solution for multithread */ } +#endif #if defined(DEBUG_SIGNAL )qemu_printf ("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n" , pc, address, is_write, *(unsigned long *)old_set);
@@ -103,7 +109,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address, }/* see if it is an MMU fault */ - ret =cpu_handle_mmu_fault (env, address, is_write,MMU_USER_IDX ); + ret =cpu_handle_mmu_fault (cpu_single_env, address, is_write, +MMU_USER_IDX );if (ret <0 ) {return 0 ;/* not an MMU fault */ }
@@ -115,13 +122,13 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,if (tb) {/* the PC is inside the translated code. It means that we have a virtual CPU fault */ -cpu_restore_state (tb, env, pc); +cpu_restore_state (tb, cpu_single_env, pc); }/* we restore the process signal mask as the sigreturn should do it (XXX: use sigsetjmp) */ sigprocmask (SIG_SETMASK , old_set,NULL ); -exception_action (env); +exception_action (cpu_single_env);/* never comes here */ return 1 ;
Signature · 1 line
1.7.2.5
PATCH 5/5 configure: disable a few Clang compiler warnings
In reply to [email protected]From: Blue Swirl <[email protected]>
Clang compiler warns about a few constructs in QEMU code. It's possible to avoid those but that needs more work.
Suppress some warnings for Clang compiler. -Wno-unused-value would conflict with GCC.
--- configure | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
configure +4 -1
@@ -1154,17 +1154,20 @@ if test -z "$werror" ; then fi fi +# GCC flags gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags" +# Clang flags +clang_flags="-Wno-initializer-overrides -Wno-self-assign -Wno-constant-conversion" if test "$werror" = "yes" ; then gcc_flags="-Werror $gcc_flags" fi cat > $TMPC << EOF int main(void) { return 0; } EOF -for flag in $gcc_flags; do +for flag in $gcc_flags $clang_flags; do if compile_prog "-Werror $flag" "" ; then QEMU_CFLAGS="$QEMU_CFLAGS $flag" fi
Signature · 1 line
1.7.2.5
Re: [Qemu-devel] [PATCH 5/5] configure: disable a few Clang compiler warnings
Am 30.07.2012 18:04, schrieb [email protected]:
Quoted 18 lines
From: Blue Swirl <[email protected]>
Clang compiler warns about a few constructs in QEMU code. It's possible to avoid those but that needs more work.
Suppress some warnings for Clang compiler. -Wno-unused-value would conflict with GCC.
Signed-off-by Blue Swirl <[email protected]>--- configure | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)configure +0
-0
configure if test -z "$werror" ; then · line 1163
Quoted 10 lines
configure +3
-0@@ -1154,17 +1154,20 @@ if test -z "$werror" ; then fi fi +# GCC flags gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags" +# Clang flags +clang_flags="-Wno-initializer-overrides -Wno-self-assign -Wno-constant-conversion"
I'd prefer getting these warnings (and not having them disabled) for compilations without -Werror ("$werror" = "no").
Regards,
Stefan W.
Quoted 12 lines
configure +1
-1if test "$werror" = "yes" ; then gcc_flags="-Werror $gcc_flags" fi cat > $TMPC << EOF int main(void) { return 0; } EOF -for flag in $gcc_flags; do +for flag in $gcc_flags $clang_flags; do if compile_prog "-Werror $flag" "" ; then QEMU_CFLAGS="$QEMU_CFLAGS $flag" fi
Re: [Qemu-devel] [PATCH 5/5] configure: disable a few Clang compiler warnings
Quoted 28 lines · Stefan Weil
Am 30.07.2012 18:04, schrieb [email protected]:
Quoted 26 lines
From: Blue Swirl <[email protected]>
Clang compiler warns about a few constructs in QEMU code. It's possible to avoid those but that needs more work.
Suppress some warnings for Clang compiler. -Wno-unused-value would conflict with GCC.
Signed-off-by Blue Swirl <[email protected]>--- configure | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)configure +0
-1@@ -1154,17 +1154,20 @@ if test -z "$werror" ; then fi fi +# GCC flags gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs$gcc_flags"
Quoted 8 lines
configure
Quoted 4 lines
gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
configure +2
-1+# Clang flags +clang_flags="-Wno-initializer-overrides -Wno-self-assign -Wno-constant-conversion"I'd prefer getting these warnings (and not having them disabled) for compilations without -Werror ("$werror" = "no").
On second thought the patch makes little sense, I'll drop it. I was trying to get everything compiled with -Werror, but that's impossible anyway with AREG0 conversion unfinished.
Quoted 21 lines
Regards,
Stefan W.
Quoted 12 lines
if test "$werror" = "yes" ; then
gcc_flags="-Werror $gcc_flags"
fi
cat > $TMPC << EOF
int main(void) { return 0; }
EOFconfigure +1
-1-for flag in $gcc_flags; do +for flag in $gcc_flags $clang_flags; do if compile_prog "-Werror $flag" "" ; then QEMU_CFLAGS="$QEMU_CFLAGS $flag" fi