| Message ID | 20170410060655.32289-1-nikunj@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes: > While the configure script generates TARGET_SUPPORTS_MTTCG define, one > of the define is cpus.c is checking wrong name: TARGET_SUPPORT_MTTCG > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > cpus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cpus.c b/cpus.c > index 68fdbc4..58d90aa 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) > } else if (use_icount) { > error_setg(errp, "No MTTCG when icount is enabled"); > } else { > -#ifndef TARGET_SUPPORT_MTTCG > +#ifndef TARGET_SUPPORTS_MTTCG > error_report("Guest not yet converted to MTTCG - " > "you may get unexpected results"); > #endif Opps that's embarrassing. Applied to my tree, I'll be sending a pullreq today -- Alex Bennée
On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: > While the configure script generates TARGET_SUPPORTS_MTTCG define, one > of the define is cpus.c is checking wrong name: TARGET_SUPPORT_MTTCG > > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > cpus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cpus.c b/cpus.c > index 68fdbc4..58d90aa 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) > } else if (use_icount) { > error_setg(errp, "No MTTCG when icount is enabled"); > } else { > -#ifndef TARGET_SUPPORT_MTTCG > +#ifndef TARGET_SUPPORTS_MTTCG This sort of thing is why glibc moved to using -Wundef. It would be a huge amount of work to convert our existing sources, but it would probably pay off in the long run. r~
On 10 April 2017 at 18:26, Richard Henderson <rth@twiddle.net> wrote: > On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: >> --- a/cpus.c >> +++ b/cpus.c >> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) >> } else if (use_icount) { >> error_setg(errp, "No MTTCG when icount is enabled"); >> } else { >> -#ifndef TARGET_SUPPORT_MTTCG >> +#ifndef TARGET_SUPPORTS_MTTCG > > > This sort of thing is why glibc moved to using -Wundef. > > It would be a huge amount of work to convert our existing sources, but it > would probably pay off in the long run. We already build with -Wundef... thanks -- PMM
On 04/10/2017 10:29 AM, Peter Maydell wrote: > On 10 April 2017 at 18:26, Richard Henderson <rth@twiddle.net> wrote: >> On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: >>> --- a/cpus.c >>> +++ b/cpus.c >>> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) >>> } else if (use_icount) { >>> error_setg(errp, "No MTTCG when icount is enabled"); >>> } else { >>> -#ifndef TARGET_SUPPORT_MTTCG >>> +#ifndef TARGET_SUPPORTS_MTTCG >> >> >> This sort of thing is why glibc moved to using -Wundef. >> >> It would be a huge amount of work to convert our existing sources, but it >> would probably pay off in the long run. > > We already build with -Wundef... Ah, but then you also have to stop using #ifdef and #ifndef, and use only #if. Thus my incomplete comment above about conversion. r~
On Mon, 10 Apr 2017 18:29:57 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 10 April 2017 at 18:26, Richard Henderson <rth@twiddle.net> wrote: > > On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: > >> --- a/cpus.c > >> +++ b/cpus.c > >> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) > >> } else if (use_icount) { > >> error_setg(errp, "No MTTCG when icount is enabled"); > >> } else { > >> -#ifndef TARGET_SUPPORT_MTTCG > >> +#ifndef TARGET_SUPPORTS_MTTCG > > > > > > This sort of thing is why glibc moved to using -Wundef. > > > > It would be a huge amount of work to convert our existing sources, but it > > would probably pay off in the long run. > > We already build with -Wundef... > From the gcc info page: '-Wundef' Warn if an undefined identifier is evaluated in an '#if' directive. and BTW, isn't the purpose of #ifndef precisely to detect that the identifier is undefined ? > thanks > -- PMM >
On 04/10/2017 10:44 AM, Greg Kurz wrote: > On Mon, 10 Apr 2017 18:29:57 +0100 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On 10 April 2017 at 18:26, Richard Henderson <rth@twiddle.net> wrote: >>> On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: >>>> --- a/cpus.c >>>> +++ b/cpus.c >>>> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) >>>> } else if (use_icount) { >>>> error_setg(errp, "No MTTCG when icount is enabled"); >>>> } else { >>>> -#ifndef TARGET_SUPPORT_MTTCG >>>> +#ifndef TARGET_SUPPORTS_MTTCG >>> >>> >>> This sort of thing is why glibc moved to using -Wundef. >>> >>> It would be a huge amount of work to convert our existing sources, but it >>> would probably pay off in the long run. >> >> We already build with -Wundef... >> > > From the gcc info page: > > '-Wundef' > Warn if an undefined identifier is evaluated in an '#if' directive. > > and BTW, isn't the purpose of #ifndef precisely to detect that the > identifier is undefined ? Yes, but it also has the typo problem above, whereas #if !TARGET_SUPPORTS_MTTCG plus -Wundef would have caught that error. r~
On Mon, 10 Apr 2017 10:49:12 -0700 Richard Henderson <rth@twiddle.net> wrote: > On 04/10/2017 10:44 AM, Greg Kurz wrote: > > On Mon, 10 Apr 2017 18:29:57 +0100 > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > >> On 10 April 2017 at 18:26, Richard Henderson <rth@twiddle.net> wrote: > >>> On 04/09/2017 11:06 PM, Nikunj A Dadhania wrote: > >>>> --- a/cpus.c > >>>> +++ b/cpus.c > >>>> @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) > >>>> } else if (use_icount) { > >>>> error_setg(errp, "No MTTCG when icount is enabled"); > >>>> } else { > >>>> -#ifndef TARGET_SUPPORT_MTTCG > >>>> +#ifndef TARGET_SUPPORTS_MTTCG > >>> > >>> > >>> This sort of thing is why glibc moved to using -Wundef. > >>> > >>> It would be a huge amount of work to convert our existing sources, but it > >>> would probably pay off in the long run. > >> > >> We already build with -Wundef... > >> > > > > From the gcc info page: > > > > '-Wundef' > > Warn if an undefined identifier is evaluated in an '#if' directive. > > > > and BTW, isn't the purpose of #ifndef precisely to detect that the > > identifier is undefined ? > > Yes, but it also has the typo problem above, whereas > > #if !TARGET_SUPPORTS_MTTCG > > plus -Wundef would have caught that error. > Turning such #ifdef/#ifndef to #if/#if ! would indeed make sense, but we'd still have to be careful that every new addition follows the rule. > > r~
diff --git a/cpus.c b/cpus.c index 68fdbc4..58d90aa 100644 --- a/cpus.c +++ b/cpus.c @@ -202,7 +202,7 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp) } else if (use_icount) { error_setg(errp, "No MTTCG when icount is enabled"); } else { -#ifndef TARGET_SUPPORT_MTTCG +#ifndef TARGET_SUPPORTS_MTTCG error_report("Guest not yet converted to MTTCG - " "you may get unexpected results"); #endif
While the configure script generates TARGET_SUPPORTS_MTTCG define, one of the define is cpus.c is checking wrong name: TARGET_SUPPORT_MTTCG Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- cpus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)