[libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases

Daniel P. Berrangé posted 14 patches 7 years, 2 months ago
[libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago
Cast away enum type for libxl schedular constants since we don't want to
cover all of them and don't want build to break when new ones are added.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libxl/libxl_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index be11134fb2..4b52de36f5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
 
     if (nparams)
         *nparams = 0;
-    switch (sched_id) {
+    switch ((int)sched_id) {
     case LIBXL_SCHEDULER_SEDF:
         name = "sedf";
         break;
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Posted by Martin Kletzander 7 years, 2 months ago
On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
>Cast away enum type for libxl schedular constants since we don't want to

s/schedular/scheduler/

>cover all of them and don't want build to break when new ones are added.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/libxl/libxl_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>index be11134fb2..4b52de36f5 100644
>--- a/src/libxl/libxl_driver.c
>+++ b/src/libxl/libxl_driver.c
>@@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
>
>     if (nparams)
>         *nparams = 0;
>-    switch (sched_id) {
>+    switch ((int)sched_id) {

We should get consistent with the spacing after cast and add it to HACKING.
This way I can't tell you to add a space there =)

>     case LIBXL_SCHEDULER_SEDF:
>         name = "sedf";
>         break;
>-- 
>2.14.3
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Wed, Feb 21, 2018 at 10:35:04AM +0100, Martin Kletzander wrote:
> On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
> > Cast away enum type for libxl schedular constants since we don't want to
> 
> s/schedular/scheduler/
> 
> > cover all of them and don't want build to break when new ones are added.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > src/libxl/libxl_driver.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index be11134fb2..4b52de36f5 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
> > 
> >     if (nparams)
> >         *nparams = 0;
> > -    switch (sched_id) {
> > +    switch ((int)sched_id) {
> 
> We should get consistent with the spacing after cast and add it to HACKING.
> This way I can't tell you to add a space there =)

Adding a space between a typecast and a variable is a-typical in general,
so I don't see why we should do it for switch() statements.

> 
> >     case LIBXL_SCHEDULER_SEDF:
> >         name = "sedf";
> >         break;
> > -- 
> > 2.14.3
> > 
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list



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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Posted by Martin Kletzander 7 years, 2 months ago
On Wed, Feb 21, 2018 at 09:39:07AM +0000, Daniel P. Berrangé wrote:
>On Wed, Feb 21, 2018 at 10:35:04AM +0100, Martin Kletzander wrote:
>> On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
>> > Cast away enum type for libxl schedular constants since we don't want to
>>
>> s/schedular/scheduler/
>>
>> > cover all of them and don't want build to break when new ones are added.
>> >
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> > ---
>> > src/libxl/libxl_driver.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> > index be11134fb2..4b52de36f5 100644
>> > --- a/src/libxl/libxl_driver.c
>> > +++ b/src/libxl/libxl_driver.c
>> > @@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
>> >
>> >     if (nparams)
>> >         *nparams = 0;
>> > -    switch (sched_id) {
>> > +    switch ((int)sched_id) {
>>
>> We should get consistent with the spacing after cast and add it to HACKING.
>> This way I can't tell you to add a space there =)
>
>Adding a space between a typecast and a variable is a-typical in general,
>so I don't see why we should do it for switch() statements.
>

Well, I didn't use to do it, but nowadays I do since it's more clearly readable.
Plus it looks like majority casts in our code use spaces.  I'll try create a
syntax-check and make some statistics, we'll see if that works or not.  I didn't
mean to pollute this series with unrelated discussion.

>>
>> >     case LIBXL_SCHEDULER_SEDF:
>> >         name = "sedf";
>> >         break;
>> > --
>> > 2.14.3
>> >
>> > --
>> > libvir-list mailing list
>> > libvir-list@redhat.com
>> > https://www.redhat.com/mailman/listinfo/libvir-list
>
>
>
>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 :|
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Posted by John Ferlan 7 years, 2 months ago

On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Cast away enum type for libxl schedular constants since we don't want to
> cover all of them and don't want build to break when new ones are added.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/libxl/libxl_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list