ZhouPeng writes ("Re: [Xen-devel] [PATCH] Play with spice for
xen-upstream-qemu on upstream Xen"):
> [Ian Jackson:]
> > Did you add the ": " after testing ? If so then perhaps the existing
> > logging functions are wrong.
>
> The output is like this,
> libxl: something : at least one of the spiceport ....
That's strange.
> It is
> void libxl__logv(...)
> libxl__logv
> xtl_log(ctx->lg, msglevel, errnoval, "libxl",
> "%s%s%s%s" "%s",
> fileline, func&&file?":":"", func?func:"",
> func||file?" ":"", // here
Yes, I think this is arguably wrong.
> output the msg.
> It use a space not ': ' between func name and log msg.
> So no bug
> But I feel the format like below is more clear
> void libxl__logv(...)
> libxl__logv
> xtl_log(ctx->lg, msglevel, errnoval, "libxl",
> "%s%s%s%s" "%s",
> fileline, func&&file?":":"", func?func:"",
> func||file?": ":"", // here
>
> If you reply to agree to use ': ' instead of ' ',
> I will send a little patch for this.
Yes, I agree. I will make this change myself as it's just one
character :-). Thanks for digging.
> Any way, I agree with you to trim the header ':' like below
> ": at least one of the spiceport or tls_port must be provided" to
> "at least one of the spiceport or tls_port must be provided"
Right, that's what I meant.
> > Secondly, your patch has a lot of rather long lines in new code. Can
> > you please try to keep your lines down to 75 characters (or 80 if you
> > absolutely must) ?
>
> Fixed in this patch.
Good, thanks.
> There are many lines up to 80 characters in libxl.idl
> and even in libxl_dm.c, that's why I turn a blind eye
> to libxl.idl in my last patch.
Right, yes, that's fine. Well it would be nicer if libxl.idl were
narrower but I don't propose to argue about that right now :-).
I have applied your patch. Thanks for your contribution.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home