Re: [Xen-devel] [PATCH v2] tools/libxl: libxl_get_scheduler should return an int

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-03-21 at 14:28 +0000, Ian Jackson wrote:
> Julien Grall writes ("[PATCH v2] tools/libxl: libxl_get_scheduler should 
> return an int"):
> > libxl_get_scheduler returns either a valid value in enum range or 
> > ERROR_FAIL.
> > 
> > As function return type is an enum, chekcing if the value is negative will
> > be always false. Therefore both GCC and clang will never go to the error
> > case.
> ...
> 
> Thanks.
> 
> The libxl part is correct, but I 

did you intend to say anything other than "have some comments on the xl
part" here?

> 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 8990020..7c73ee0 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -4819,7 +4819,7 @@ int main_vcpuset(int argc, char **argv)
> >  static void output_xeninfo(void)
> >  {
> >      const libxl_version_info *info;
> > -    libxl_scheduler sched;
> > +    int sched;
> 
> OK...
> 
> >      if (!(info = libxl_get_version_info(ctx))) {
> >          fprintf(stderr, "libxl_get_version_info failed.\n");
> > @@ -6706,10 +6706,12 @@ int main_cpupoolcreate(int argc, char **argv)
> >              goto out_cfg;
> >          }
> >      } else {
> > -        if ((sched = libxl_get_scheduler(ctx)) < 0) {
> > +
> > +        if ((ret = libxl_get_scheduler(ctx)) < 0) {
> >              fprintf(stderr, "get_scheduler sysctl failed.\n");
> >              goto out_cfg;
> >          }
> > +        sched = ret;
> 
> But then I don't understand why you changed this too.  Either of these
> changes would suffice by itself, and the former is marginally less
> fiddly.
> 
> Thanks,
> Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel