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