Change Reader::fields() to return an iterator instead of a slice.

src/lib.rs

@@ -59,6 +59,7 @@
 //!   they are now spelled in all uppercase (e.g., `Context::TIFF`).
 //! * `Value` became a self-contained type.  The structures of `Value::Ascii`
 //!   and `Value::Undefined` have been changed to use Vec<u8> instead of &[u8].
+//! * `Reader::fields` now returns an iterator instead of a slice.
 
 pub use error::Error;
 pub use jpeg::get_exif_attr as get_exif_attr_from_jpeg;

src/reader.rs

@@ -24,16 +24,15 @@
 // SUCH DAMAGE.
 //
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::io;
 use std::io::Read;
-use std::mem;
 
 use crate::error::Error;
 use crate::jpeg;
 use crate::tag::Tag;
 use crate::tiff;
-use crate::tiff::{Field, In, ProvideUnit};
+use crate::tiff::{Field, IfdEntry, In, ProvideUnit};
 
 /// The `Reader` struct reads a JPEG or TIFF image,
 /// parses the Exif attributes in it, and holds the results.
@@ -63,12 +62,11 @@ use crate::tiff::{Field, In, ProvideUnit};
 pub struct Reader {
     // TIFF data.
     buf: Vec<u8>,
-    // Exif fields.
-    fields: Vec<Field>,
+    // Exif fields.  `BTreeMap` is used so that `Reader::field` returns
+    // the fields in a stable order.
+    entries: BTreeMap<(In, Tag), IfdEntry>,
     // True if the TIFF data is little endian.
     little_endian: bool,
-    // HashMap to find a field quickly.
-    field_map: HashMap<(Tag, In), &'static Field>,
 }
 
 impl Reader {
@@ -89,20 +87,14 @@ impl Reader {
             return Err(Error::InvalidFormat("Unknown image format"));
         }
 
-        let (fields, le) = tiff::parse_exif_compat03(&buf)?;
-
-        // Initialize the HashMap of all fields.
-        let mut field_map = HashMap::new();
-        for f in &fields {
-            field_map.insert((f.tag, f.ifd_num),
-                             unsafe { mem::transmute::<&Field, &Field>(f) });
-        }
+        let (entries, le) = tiff::parse_exif(&buf)?;
+        let entries = entries.into_iter()
+            .map(|e| (e.ifd_num_tag(), e)).collect();
 
         Ok(Reader {
             buf: buf,
-            fields: fields,
+            entries: entries,
             little_endian: le,
-            field_map: field_map
         })
     }
 
@@ -114,8 +106,9 @@ impl Reader {
 
     /// Returns a slice of Exif fields.
     #[inline]
-    pub fn fields<'a>(&'a self) -> &[Field] {
-        &self.fields
+    pub fn fields<'a>(&'a self) -> impl ExactSizeIterator<Item = &'a Field> {
+        self.entries.values()
+            .map(move |e| e.ref_field(&self.buf, self.little_endian))
     }
 
     /// Returns true if the TIFF data is in the little-endian byte order.
@@ -128,7 +121,8 @@ impl Reader {
     /// and the IFD number.
     #[inline]
     pub fn get_field(&self, tag: Tag, ifd_num: In) -> Option<&Field> {
-        self.field_map.get(&(tag, ifd_num)).map(|&f| f)
+        self.entries.get(&(ifd_num, tag))
+            .map(|e| e.ref_field(&self.buf, self.little_endian))
     }
 }
 
@@ -145,54 +139,6 @@ mod tests {
     use crate::value::Value;
     use super::*;
 
-    static TIFF_ASCII: &'static [u8] =
-        b"MM\0\x2a\0\0\0\x08\0\x01\x01\x0e\0\x02\0\0\0\x04ABC\0\0\0\0\0";
-
-    // Test if moving a `Reader` does not invalidate the references in it.
-    // The referer is in the heap and does not move, so this test is not
-    // so meaningful.
-    #[test]
-    fn move_reader() {
-        let r1 = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap();
-        let ptr = &r1 as *const _;
-        move_in_and_drop(r1, ptr);
-
-        let (r2, ptr) = move_out_and_drop();
-        assert!(ptr != &r2 as *const _, "not moved");
-        check_abc(r2.fields());
-
-        box_and_drop();
-    }
-
-    #[inline(never)]
-    fn move_in_and_drop(r1: Reader, ptr: *const Reader) {
-        assert!(ptr != &r1 as *const _, "not moved");
-        check_abc(r1.fields());
-    }
-
-    #[inline(never)]
-    fn move_out_and_drop() -> (Reader, *const Reader) {
-        let r2 = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap();
-        let ptr = &r2 as *const _;
-        (r2, ptr)
-    }
-
-    fn box_and_drop() {
-        let r = Reader::new(&mut BufReader::new(TIFF_ASCII)).unwrap();
-        let ptr = &r as *const _;
-        let b = Box::new(r);
-        assert!(ptr != &*b as *const _, "not moved");
-        check_abc(b.fields());
-    }
-
-    fn check_abc(fields: &[Field]) {
-        if let Value::Ascii(ref v) = fields[0].value {
-            assert_eq!(*v, vec![b"ABC"]);
-        } else {
-            panic!("TIFF ASCII field is expected");
-        }
-    }
-
     #[test]
     fn get_field() {
         let file = File::open("tests/yaminabe.tif").unwrap();

src/tag.rs

@@ -51,7 +51,7 @@ use crate::util::atou16;
 // PartialEq and Eq need to be _automatically derived_ for Tag to
 // emulate structural equivalency.
 // <https://github.com/rust-lang/rfcs/pull/1445>
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Tag(pub Context, pub u16);
 
 impl Tag {
@@ -110,7 +110,7 @@ impl fmt::Display for Tag {
 /// A type that indicates how a tag number is interpreted.
 // This type is not an enum to keep API compatibilities when
 // new contexts are introduced.
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct Context(std::num::NonZeroU16);
 
 macro_rules! generate_context {

src/tiff.rs

@@ -53,6 +53,21 @@ pub struct IfdEntry {
 }
 
 impl IfdEntry {
+    pub fn ifd_num_tag(&self) -> (In, Tag) {
+        if self.field.is_fixed() {
+            let field = self.field.get_ref();
+            (field.ifd_num, field.tag)
+        } else {
+            let field = self.field.get_mut();
+            (field.ifd_num, field.tag)
+        }
+    }
+
+    pub fn ref_field<'a>(&'a self, data: &[u8], le: bool) -> &'a Field {
+        self.parse(data, le);
+        self.field.get_ref()
+    }
+
     fn into_field(self, data: &[u8], le: bool) -> Field {
         self.parse(data, le);
         self.field.into_inner()
@@ -106,7 +121,7 @@ pub struct Field {
 /// assert_eq!(In::PRIMARY.index(), 0);
 /// assert_eq!(In::THUMBNAIL.index(), 1);
 /// ```
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct In(pub u16);
 
 impl In {
@@ -537,9 +552,10 @@ mod tests {
     fn unknown_field() {
         let data = b"MM\0\x2a\0\0\0\x08\
                      \0\x01\x01\0\xff\xff\0\0\0\x01\0\x14\0\0\0\0\0\0";
-        let (v, _) = parse_exif_compat03(data).unwrap();
+        let (v, le) = parse_exif(data).unwrap();
         assert_eq!(v.len(), 1);
-        assert_pat!(v[0].value, Value::Unknown(0xffff, 1, 0x12));
+        assert_pat!(v[0].ref_field(data, le).value,
+                    Value::Unknown(0xffff, 1, 0x12));
     }
 
     #[test]

tests/rwrcmp.rs

@@ -111,13 +111,13 @@ fn rwr_compare<P>(path: P) where P: AsRef<Path> {
     let reader2 = Reader::new(&mut &out[..]).unwrap();
 
     // Sort the fields (some files have wrong tag order).
-    let mut fields1 = reader1.fields().iter().map(|f| f).collect::<Vec<_>>();
-    fields1.sort_by_key(|f| f.tag.number());
-    let mut fields2 = reader2.fields().iter().map(|f| f).collect::<Vec<_>>();
-    fields2.sort_by_key(|f| f.tag.number());
+    let mut fields1 = reader1.fields().collect::<Vec<_>>();
+    fields1.sort_by_key(|f| (f.ifd_num, f.tag));
+    let mut fields2 = reader2.fields().collect::<Vec<_>>();
+    fields2.sort_by_key(|f| (f.ifd_num, f.tag));
 
     // Compare.
-    assert_eq!(reader1.fields().len(), reader2.fields().len());
+    assert_eq!(fields1.len(), fields2.len());
     for (f1, f2) in fields1.iter().zip(fields2.iter()) {
         assert_eq!(f1.tag, f2.tag);
         assert_eq!(f1.ifd_num, f2.ifd_num);