bread-graphics/breadx

window.change_property crashed with String as AsByteSequence

Dushistov opened this issue · 3 comments

artificial example:

    let prop_atom = conn.intern_atom_immediate_async("PENGUIN", false).await?;
    window
        .change_property_async(
            &mut conn,
            prop_atom,
            PropertyType::String,
            PropertyFormat::Eight,
            PropMode::Append,
            &["aaaa".to_string()],
        )
        .await?;

The code looks like expect that fixed length structure with size >= package size is supplied:

 let mut data_bytes: Vec<u8> = iter::repeat(0)
            .take(mem::size_of::<T>() * data.len())
            .collect();
        let len = data.iter().fold(0, |index, item| {
            item.as_bytes(&mut data_bytes[index..]) + index
        });

But String is obviously not want this code expect.
May be add method that take single &T instead of &[T]?
Plus some extra trait for all fixed structure, so code example above won't compile?

Also it would be nice to make possible supply atom instead of property_type: PropertyType,, I need supply UTF8_STRING atom to this method.

Further testing show that AsByteSequence for String also crashed.
Because of bytes.copy_from_slice(x) expect that bytes.len() == x.len(),
so copy_from_slice failed either bytes[self.len()] = 0 failed.
Testing showing that Xorg works with string without leading zero just fine.
Not sure why here \0.

impl AsByteSequence for String {  
    fn as_bytes(&self, bytes: &mut [u8]) -> usize {
        bytes.copy_from_slice(String::as_bytes(self));
        bytes[self.len()] = 0;
        self.len() + 1
    }

As a fix, you can do:

    window
        .change_property_async(
            &mut conn,
            prop_atom,
            PropertyType::String,
            PropertyFormat::Eight,
            PropMode::Append,
            "aaaa".as_bytes(),
        )
        .await?;

When I was writing the AsByteSequence impl for String, I had reading Strings in mind, not writing them. Perhaps I should split AsByteSequence into two traits come 3.0.

With the new protocol system, this should be resolved now.