georust / gdal

Rust bindings for GDAL
https://crates.io/crates/gdal
MIT License
359 stars 94 forks source link

`Dataset::rasterband` doesn't follow Rust mutable reference rules. #450

Open metasim opened 1 year ago

metasim commented 1 year ago

Currently, the following compiles, but the test fails:

#[test]
fn test_mut() {
    let dataset = Dataset::open(fixture("tinymarble.tif")).unwrap();
    let b1 = dataset.rasterband(1).unwrap();
    let before_desc = b1.description().unwrap();

    {
        let mut also_b1 = dataset.rasterband(1).unwrap();
        // Arbitrary mutation
        also_b1.set_description("smash").unwrap();
    }

    let after_desc = b1.description().unwrap();

    assert_eq!(before_desc, after_desc);
}

Given we never request a mutable reference to Dataset, one could argue you should never be able to get a mutable reference to an owned, constituent component of the Dataset, as we have done here.

jdroenner commented 1 year ago

yes thats right. It was probably overlooked at some point. We could introduce something as an OwnedRasterband.... However you don't even need to mutate something on purpose. Using RasterIO can (and will in many cases) modify some global variables.

metasim commented 1 year ago

This is how I'd propose we solve it (assumes foreign-types crate applied to Rasterband.