bread-graphics/breadx

Bug in SYSTEMCOUNTER

Opened this issue · 2 comments

As described in https://bugs.freedesktop.org/show_bug.cgi?id=23403 the SYSTEMCOUNTER struct in sync extension is bugged.
It needs a global pad instead of a last-field pad.

The solution is to substitute index += buffer_pad(block_len, 4) with index += buffer_pad(index, 4) on these lines
https://github.com/bread-graphics/breadx/blob/master/src/auto/sync.rs#L88
https://github.com/bread-graphics/breadx/blob/master/src/auto/sync.rs#L104
So consequently the size method needs a rewrite too.

The bug is still open since 2009, you can be the first one implementing the protocol in the right way (except for libx11).

partly off-topic, feel free to delete this comment; at least this indicates that the XML is correct

I was just passing by (looking for something else), noticed the following and wondered what x11rb does (since the bug is about C compiler padding, x11rb should not be affected):

you can be the first one implementing the protocol in the right way (except for libx11).

Here is a port of the example from the above bug:

use x11rb::rust_connection::RustConnection;
use x11rb::protocol::sync::{self, ConnectionExt as _};

fn to_i64(input: sync::Int64) -> i64 {
    (i64::from(input.hi) << 32) | i64::from(input.lo)
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let (conn, _screen) = RustConnection::connect(None)?;
    let (major, minor) = sync::X11_XML_VERSION;
    conn.sync_initialize(major as _, minor as _)?.reply()?;
    let list = conn.sync_list_system_counters()?.reply()?;
    for counter in list.counters {
        let name = std::str::from_utf8(&counter.name)?;
        println!("counter {:#x}: {} (resolution: {})", counter.counter, name, to_i64(counter.resolution));
    }
    Ok(())
}

Output:

counter 0x772: DEVICEIDLETIME 15 (resolution: 4)
counter 0x771: DEVICEIDLETIME 14 (resolution: 4)
counter 0x770: DEVICEIDLETIME 13 (resolution: 4)
counter 0x76f: DEVICEIDLETIME 12 (resolution: 4)
counter 0x76e: DEVICEIDLETIME 11 (resolution: 4)
counter 0x76d: DEVICEIDLETIME 10 (resolution: 4)
counter 0x76c: DEVICEIDLETIME 9 (resolution: 4)
counter 0x76b: DEVICEIDLETIME 8 (resolution: 4)
counter 0x76a: DEVICEIDLETIME 7 (resolution: 4)
counter 0x769: DEVICEIDLETIME 6 (resolution: 4)
counter 0x768: DEVICEIDLETIME 5 (resolution: 4)
counter 0x767: DEVICEIDLETIME 4 (resolution: 4)
counter 0x766: DEVICEIDLETIME 3 (resolution: 4)
counter 0x765: DEVICEIDLETIME 2 (resolution: 4)
counter 0x70: IDLETIME (resolution: 4)
counter 0x6f: SERVERTIME (resolution: 4)

Looks correct to me. And I can get not the C example to work, even with the example from the bug.
xtrace on the above agrees:

000:>:0003:528: Reply to ListSystemCounters: counter={counter=0x00000772 resolution=4 name='DEVICEIDLETIME 15' },{counter=0x00000771 resolution=4 name='DEVICEIDLETIME 14' },{counter=0x00000770 resolution=4 name='DEVICEIDLETIME 13' },{counter=0x0000076f resolution=4 name='DEVICEIDLETIME 12' },{counter=0x0000076e resolution=4 name='DEVICEIDLETIME 11' },{counter=0x0000076d resolution=4 name='DEVICEIDLETIME 10' },{counter=0x0000076c resolution=4 name='DEVICEIDLETIME 9' },{counter=0x0000076b resolution=4 name='DEVICEIDLETIME 8' },{counter=0x0000076a resolution=4 name='DEVICEIDLETIME 7' },{counter=0x00000769 resolution=4 name='DEVICEIDLETIME 6' },{counter=0x00000768 resolution=4 name='DEVICEIDLETIME 5' },{counter=0x00000767 resolution=4 name='DEVICEIDLETIME 4' },{counter=0x00000766 resolution=4 name='DEVICEIDLETIME 3' },{counter=0x00000765 resolution=4 name='DEVICEIDLETIME 2' },{counter=0x00000070 resolution=4 name='IDLETIME' },{counter=0x0000006f resolution=4 name='SERVERTIME' };

Thanks for letting me know. I'm currently working on a new implementation of the code generator, and I'll be sure to include this in it.