On Tue, 2018-06-19 at 17:29 +0200, Christophe de Dinechin wrote:
> > On 7 Jun 2018, at 10:55, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote:
> >
> > On Wed, 2018-06-06 at 12:56 -0400, Frediano Ziglio wrote:
> > > >
> > > > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > > > ---
> > > > src/vdagent/x11-randr.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> > > > index 803cf73..84c75f2 100644
> > > > --- a/src/vdagent/x11-randr.c
> > > > +++ b/src/vdagent/x11-randr.c
> > > > @@ -38,6 +38,10 @@
> > > >
> > > > static int error_handler(Display *display, XErrorEvent *error)
> > > > {
> > > > + char buf[1024];
> > > > + XGetErrorText(display, error->error_code, buf, 1024);
> > > > + syslog(LOG_ERR, "X11 Error: %s", buf);
> > > > +
> > > > vdagent_x11_caught_error = 1;
> > > > return 0;
> > > > }
> > >
> > > Beside the title too slang patch looks good...
> >
> > Yeah there was a different word before this :) I can remove it, but...
> >
> > > ... but I suppose that this
> > > function is here to catch error and ignore them so why logging as
> > > error something already expected?
> >
> > Is that the purpose? I haven't realized that and it's not documented in
> > any way. Now that you tell me, I see it in the code, but it's
> > confusing.
> >
> > The concrete case I had was at x11-randr.c:840 (on current master),
> > calling XRRSetScreenSize (can't seem to find any documentation of this
> > one). So I was getting a rather unhelpful "XRRSetScreenSize failed, not
> > enough mem?" from our log, figured I'm getting some X error and got
> > even slighly more unhlepful "BadMatch" from that. Luckily I noticed the
> > comment about that a bit above the call.
> >
> > So, if the error from the XRRSetScreenSize is always "BadMatch", I it
> > can be ignored but would be more clear about it in the comments.
> >
> > Shall I add some comments instead then?
>
> Yes. Given how long your description is, they might be at least as funn as “diddly doo”.
>
> But I think that this also means we expect to ignore a specific error in a specific context, maybe be a bit more restrictive instead of hiding everything?
You mean about the specific error? As I said, I haven't found any
documentation and I faitly recall I tried quite hard. Next up dig in
the source what errors it can actually raise, but I suspect it's dumb
and just gives the BadMatch on everything.
> Christophe
> >
> > Cheers,
> > Lukas
> >
> > > Frediano
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel