Update name section parsing (#270)

* improve name section parsing

* fix tests

* some additional fixes

* little updates

* fix comments and tests

* fix identation and previous naming

* fix cmt and identation

* final ccmt fixes

* Update name_section.rs

* decrease name size

src/elements/mod.rs

@@ -53,8 +53,8 @@ pub use self::func::{Func, FuncBody, Local};
 pub use self::segment::{ElementSegment, DataSegment};
 pub use self::index_map::IndexMap;
 pub use self::name_section::{
-	NameMap, NameSection, ModuleNameSection, FunctionNameSection,
-	LocalNameSection,
+	NameMap, NameSection, ModuleNameSubsection, FunctionNameSubsection,
+	LocalNameSubsection,
 };
 pub use self::reloc_section::{
 	RelocSection, RelocationEntry,
@@ -145,6 +145,10 @@ pub enum Error {
 	InvalidSegmentFlags(u32),
 	/// Sum of counts of locals is greater than 2^32.
 	TooManyLocals,
+	/// Duplicated name subsections.
+	DuplicatedNameSubsections(u8),
+	/// Unknown name subsection type.
+	UnknownNameSubsectionType(u8),
 }
 
 impl fmt::Display for Error {
@@ -174,14 +178,16 @@ impl fmt::Display for Error {
 			Error::InconsistentMetadata =>  write!(f, "Inconsistent metadata"),
 			Error::InvalidSectionId(ref id) =>  write!(f, "Invalid section id: {}", id),
 			Error::SectionsOutOfOrder =>  write!(f, "Sections out of order"),
-			Error::DuplicatedSections(ref id) =>  write!(f, "Dupliated sections ({})", id),
+			Error::DuplicatedSections(ref id) =>  write!(f, "Duplicated sections ({})", id),
 			Error::InvalidMemoryReference(ref mem_ref) =>  write!(f, "Invalid memory reference ({})", mem_ref),
 			Error::InvalidTableReference(ref table_ref) =>  write!(f, "Invalid table reference ({})", table_ref),
 			Error::InvalidLimitsFlags(ref flags) =>  write!(f, "Invalid limits flags ({})", flags),
 			Error::UnknownFunctionForm(ref form) =>  write!(f, "Unknown function form ({})", form),
 			Error::InconsistentCode =>  write!(f, "Number of function body entries and signatures does not match"),
 			Error::InvalidSegmentFlags(n) =>  write!(f, "Invalid segment flags: {}", n),
 			Error::TooManyLocals => write!(f, "Too many locals"),
+			Error::DuplicatedNameSubsections(n) =>  write!(f, "Duplicated name subsections: {}", n),
+			Error::UnknownNameSubsectionType(n) => write!(f, "Unknown subsection type: {}", n),
 		}
 	}
 }
@@ -220,6 +226,8 @@ impl ::std::error::Error for Error {
 			Error::InconsistentCode =>  "Number of function body entries and signatures does not match",
 			Error::InvalidSegmentFlags(_) =>  "Invalid segment flags",
 			Error::TooManyLocals => "Too many locals",
+			Error::DuplicatedNameSubsections(_) =>  "Duplicated name subsections",
+			Error::UnknownNameSubsectionType(_) => "Unknown name subsections type",
 		}
 	}
 }

src/elements/module.rs

@@ -346,7 +346,7 @@ impl Module {
 		None
 	}
 
-	/// Try to parse name section in place/
+	/// Try to parse name section in place.
 	///
 	/// Corresponding custom section with proper header will convert to name sections
 	/// If some of them will fail to be decoded, Err variant is returned with the list of
@@ -370,6 +370,7 @@ impl Module {
 					}
 				} else { None }
 			} {
+				// todo: according to the spec a Wasm binary can contain only one name section
 				*self.sections.get_mut(i).expect("cannot fail because i in range 0..len; qed") = Section::Name(name_section);
 			}
 		}
@@ -523,6 +524,7 @@ impl Serialize for Module {
 		Uint32::from(self.magic).serialize(w)?;
 		Uint32::from(self.version).serialize(w)?;
 		for section in self.sections.into_iter() {
+			// todo: according to the spec the name section should appear after the data section
 			section.serialize(w)?;
 		}
 		Ok(())
@@ -753,8 +755,6 @@ mod integration_tests {
 
 	#[test]
 	fn names() {
-		use super::super::name_section::NameSection;
-
 		let module = deserialize_file("./res/cases/v1/with_names.wasm")
 			.expect("Should be deserialized")
 			.parse_names()
@@ -764,21 +764,19 @@ mod integration_tests {
 		for section in module.sections() {
 			match *section {
 				Section::Name(ref name_section) => {
-					match *name_section {
-						NameSection::Function(ref function_name_section) => {
-							assert_eq!(
-								function_name_section.names().get(0).expect("Should be entry #0"),
-								"elog"
-							);
-							assert_eq!(
-								function_name_section.names().get(11).expect("Should be entry #0"),
-								"_ZN48_$LT$pwasm_token_contract..Endpoint$LT$T$GT$$GT$3new17hc3ace6dea0978cd9E"
-							);
-
-							found_section = true;
-						},
-						_ => {},
-					}
+					let function_name_subsection = name_section
+						.functions()
+						.expect("function_name_subsection should be present");
+					assert_eq!(
+						function_name_subsection.names().get(0).expect("Should be entry #0"),
+						"elog"
+					);
+					assert_eq!(
+						function_name_subsection.names().get(11).expect("Should be entry #0"),
+						"_ZN48_$LT$pwasm_token_contract..Endpoint$LT$T$GT$$GT$3new17hc3ace6dea0978cd9E"
+					);
+
+					found_section = true;
 				},
 				_ => {},
 			}