[Xen-devel] Re: [PATCH,RFC,v2]: Introduce libxl_domain_create()

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Tue, 2010-12-21 at 15:30 +0000, Ian Campbell wrote:
> Only minor comments.
> 
> On Tue, 2010-12-21 at 15:17 +0000, Gianni Tedesco wrote:
> > @@ -246,6 +246,38 @@ enum {
> > 
> >  #define LIBXL_VERSION 0
> > 
> > +enum action_on_shutdown {
> 
> Namespace.

Ah, thanks, missed that.

> > +    LIBXL_ACTION_DESTROY,
> > +
> > +    LIBXL_ACTION_RESTART,
> > +    LIBXL_ACTION_RESTART_RENAME,
> > +
> > +    LIBXL_ACTION_PRESERVE,
> > +
> > +    LIBXL_ACTION_COREDUMP_DESTROY,
> > +    LIBXL_ACTION_COREDUMP_RESTART,
> > +};
> [...]
> > +#define MUST( call ) ({                                                 \
> > +        int must_rc = (call);                                           \
> > +        if (must_rc < 0) {                                              \
> > +            fprintf(stderr,"xl: fatal error: %s:%d, rc=%d: %s\n",       \
> > +                    __FILE__,__LINE__, must_rc, #call);                 \
> > +            goto error_out;                                             \
> > +        }                                                               \
> > +    })
> 
> I don't think the two callsites of this macro justify the obfuscation of
> having a goto in a macro, just open code it.

Yes, you're right.

> > +
> > +    if ( dom_info->console_autoconnect ) {
> > +        cb = autoconnect_console;
> > +    }else{
> 
>        } else {
> 
> > +    }else{
> 
> Some of the indentation looks weird. Could be my mail client or perhaps
> you are mixing spaces and tabs or something?

Don't think so, seems fine to me. There was something else I was working
on that was all already tabs but I don't think it's anything I touched
here.

> Ian.

Thanks

Gianni



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel