diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index 1a8872bc3e..f7a641776c 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -361,7 +361,7 @@ pub fn modify_parsed_flags_based_on_mode( Ok(modified_parsed_flags) } -pub fn assign_flag_ids<'a, I>(package: &str, parsed_flags_iter: I) -> Result> +pub fn assign_flag_ids<'a, I>(package: &str, parsed_flags_iter: I) -> Result> where I: Iterator + Clone, { @@ -371,7 +371,13 @@ where if package != pf.package() { return Err(anyhow::anyhow!("encountered a flag not in current package")); } - flag_ids.insert(pf.name().to_string(), id_to_assign); + + // put a cap on how many flags a package can contain to 65535 + if id_to_assign > u16::MAX as u32 { + return Err(anyhow::anyhow!("the number of flags in a package cannot exceed 65535")); + } + + flag_ids.insert(pf.name().to_string(), id_to_assign as u16); } Ok(flag_ids) } @@ -693,15 +699,15 @@ mod tests { let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string(); let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap(); let expected_flag_ids = HashMap::from([ - (String::from("disabled_ro"), 0_u32), - (String::from("disabled_rw"), 1_u32), - (String::from("disabled_rw_exported"), 2_u32), - (String::from("disabled_rw_in_other_namespace"), 3_u32), - (String::from("enabled_fixed_ro"), 4_u32), - (String::from("enabled_fixed_ro_exported"), 5_u32), - (String::from("enabled_ro"), 6_u32), - (String::from("enabled_ro_exported"), 7_u32), - (String::from("enabled_rw"), 8_u32), + (String::from("disabled_ro"), 0_u16), + (String::from("disabled_rw"), 1_u16), + (String::from("disabled_rw_exported"), 2_u16), + (String::from("disabled_rw_in_other_namespace"), 3_u16), + (String::from("enabled_fixed_ro"), 4_u16), + (String::from("enabled_fixed_ro_exported"), 5_u16), + (String::from("enabled_ro"), 6_u16), + (String::from("enabled_ro_exported"), 7_u16), + (String::from("enabled_rw"), 8_u16), ]); assert_eq!(flag_ids, expected_flag_ids); } diff --git a/tools/aconfig/src/storage/flag_table.rs b/tools/aconfig/src/storage/flag_table.rs index dc9bf5e70d..595217e977 100644 --- a/tools/aconfig/src/storage/flag_table.rs +++ b/tools/aconfig/src/storage/flag_table.rs @@ -58,18 +58,26 @@ impl FlagTableHeader { pub struct FlagTableNode { pub package_id: u32, pub flag_name: String, - pub flag_id: u32, + pub flag_type: u16, + pub flag_id: u16, pub next_offset: Option, pub bucket_index: u32, } impl FlagTableNode { - fn new(package_id: u32, flag_name: &str, flag_id: u32, num_buckets: u32) -> Self { + fn new( + package_id: u32, + flag_name: &str, + flag_type: u16, + flag_id: u16, + num_buckets: u32, + ) -> Self { let full_flag_name = package_id.to_string() + "/" + flag_name; let bucket_index = storage::get_bucket_index(&full_flag_name, num_buckets); Self { package_id, flag_name: flag_name.to_string(), + flag_type, flag_id, next_offset: None, bucket_index, @@ -82,6 +90,7 @@ impl FlagTableNode { let name_bytes = self.flag_name.as_bytes(); result.extend_from_slice(&(name_bytes.len() as u32).to_le_bytes()); result.extend_from_slice(name_bytes); + result.extend_from_slice(&self.flag_type.to_le_bytes()); result.extend_from_slice(&self.flag_id.to_le_bytes()); result.extend_from_slice(&self.next_offset.unwrap_or(0).to_le_bytes()); result @@ -106,7 +115,11 @@ impl FlagTable { let fid = flag_ids .get(pf.name()) .ok_or(anyhow!(format!("missing flag id for {}", pf.name())))?; - Ok(FlagTableNode::new(package.package_id, pf.name(), *fid, num_buckets)) + // all flags are boolean value at the moment, thus using the last bit. When more + // flag value types are supported, flag value type information should come from the + // parsed flag, and we will set the flag_type bit mask properly. + let flag_type = 1; + Ok(FlagTableNode::new(package.package_id, pf.name(), flag_type, *fid, num_buckets)) }) .collect::>>() } @@ -175,7 +188,7 @@ mod tests { use super::*; use crate::storage::{ group_flags_by_package, tests::parse_all_test_flags, tests::read_str_from_bytes, - tests::read_u32_from_bytes, + tests::read_u16_from_bytes, tests::read_u32_from_bytes, }; impl FlagTableHeader { @@ -200,7 +213,8 @@ mod tests { let mut node = Self { package_id: read_u32_from_bytes(bytes, &mut head)?, flag_name: read_str_from_bytes(bytes, &mut head)?, - flag_id: read_u32_from_bytes(bytes, &mut head)?, + flag_type: read_u16_from_bytes(bytes, &mut head)?, + flag_id: read_u16_from_bytes(bytes, &mut head)?, next_offset: match read_u32_from_bytes(bytes, &mut head)? { 0 => None, val => Some(val), @@ -216,13 +230,15 @@ mod tests { fn new_expected( package_id: u32, flag_name: &str, - flag_id: u32, + flag_type: u16, + flag_id: u16, next_offset: Option, bucket_index: u32, ) -> Self { Self { package_id, flag_name: flag_name.to_string(), + flag_type, flag_id, next_offset, bucket_index, @@ -306,14 +322,17 @@ mod tests { let nodes: &Vec = &flag_table.as_ref().unwrap().nodes; assert_eq!(nodes.len(), 8); - assert_eq!(nodes[0], FlagTableNode::new_expected(0, "enabled_ro", 1, None, 0)); - assert_eq!(nodes[1], FlagTableNode::new_expected(0, "enabled_rw", 2, Some(150), 1)); - assert_eq!(nodes[2], FlagTableNode::new_expected(1, "disabled_ro", 0, None, 1)); - assert_eq!(nodes[3], FlagTableNode::new_expected(2, "enabled_ro", 1, None, 5)); - assert_eq!(nodes[4], FlagTableNode::new_expected(1, "enabled_fixed_ro", 1, Some(235), 7)); - assert_eq!(nodes[5], FlagTableNode::new_expected(1, "enabled_ro", 2, None, 7)); - assert_eq!(nodes[6], FlagTableNode::new_expected(2, "enabled_fixed_ro", 0, None, 9)); - assert_eq!(nodes[7], FlagTableNode::new_expected(0, "disabled_rw", 0, None, 15)); + assert_eq!(nodes[0], FlagTableNode::new_expected(0, "enabled_ro", 1, 1, None, 0)); + assert_eq!(nodes[1], FlagTableNode::new_expected(0, "enabled_rw", 1, 2, Some(150), 1)); + assert_eq!(nodes[2], FlagTableNode::new_expected(1, "disabled_ro", 1, 0, None, 1)); + assert_eq!(nodes[3], FlagTableNode::new_expected(2, "enabled_ro", 1, 1, None, 5)); + assert_eq!( + nodes[4], + FlagTableNode::new_expected(1, "enabled_fixed_ro", 1, 1, Some(235), 7) + ); + assert_eq!(nodes[5], FlagTableNode::new_expected(1, "enabled_ro", 1, 2, None, 7)); + assert_eq!(nodes[6], FlagTableNode::new_expected(2, "enabled_fixed_ro", 1, 0, None, 9)); + assert_eq!(nodes[7], FlagTableNode::new_expected(0, "disabled_rw", 1, 0, None, 15)); } #[test] diff --git a/tools/aconfig/src/storage/mod.rs b/tools/aconfig/src/storage/mod.rs index 76835e015d..a28fccd917 100644 --- a/tools/aconfig/src/storage/mod.rs +++ b/tools/aconfig/src/storage/mod.rs @@ -56,6 +56,8 @@ pub struct FlagPackage<'a> { pub package_id: u32, pub flag_names: HashSet<&'a str>, pub boolean_flags: Vec<&'a ProtoParsedFlag>, + // offset of the first boolean flag in this flag package with respect to the start of + // boolean flag value array in the flag value file pub boolean_offset: u32, } @@ -95,12 +97,11 @@ where } // calculate package flag value start offset, in flag value file, each boolean - // is stored as two bytes, the first byte will be the flag value. the second - // byte is flag info byte, which is a bitmask to indicate the status of a flag + // is stored as a single byte let mut boolean_offset = 0; for p in packages.iter_mut() { p.boolean_offset = boolean_offset; - boolean_offset += 2 * p.boolean_flags.len() as u32; + boolean_offset += p.boolean_flags.len() as u32; } packages @@ -135,6 +136,13 @@ mod tests { use super::*; use crate::Input; + /// Read and parse bytes as u16 + pub fn read_u16_from_bytes(buf: &[u8], head: &mut usize) -> Result { + let val = u16::from_le_bytes(buf[*head..*head + 2].try_into()?); + *head += 2; + Ok(val) + } + /// Read and parse bytes as u32 pub fn read_u32_from_bytes(buf: &[u8], head: &mut usize) -> Result { let val = u32::from_le_bytes(buf[*head..*head + 4].try_into()?); @@ -218,13 +226,13 @@ mod tests { assert!(packages[1].flag_names.contains("enabled_ro")); assert!(packages[1].flag_names.contains("disabled_ro")); assert!(packages[1].flag_names.contains("enabled_fixed_ro")); - assert_eq!(packages[1].boolean_offset, 6); + assert_eq!(packages[1].boolean_offset, 3); assert_eq!(packages[2].package_name, "com.android.aconfig.storage.test_4"); assert_eq!(packages[2].package_id, 2); assert_eq!(packages[2].flag_names.len(), 2); assert!(packages[2].flag_names.contains("enabled_ro")); assert!(packages[2].flag_names.contains("enabled_fixed_ro")); - assert_eq!(packages[2].boolean_offset, 12); + assert_eq!(packages[2].boolean_offset, 6); } } diff --git a/tools/aconfig/src/storage/package_table.rs b/tools/aconfig/src/storage/package_table.rs index 1a3bbc3eb0..0ce13493b5 100644 --- a/tools/aconfig/src/storage/package_table.rs +++ b/tools/aconfig/src/storage/package_table.rs @@ -57,6 +57,8 @@ impl PackageTableHeader { pub struct PackageTableNode { pub package_name: String, pub package_id: u32, + // offset of the first boolean flag in this flag package with respect to the start of + // boolean flag value array in the flag value file pub boolean_offset: u32, pub next_offset: Option, pub bucket_index: u32, @@ -249,7 +251,7 @@ mod tests { let first_node_expected = PackageTableNode { package_name: String::from("com.android.aconfig.storage.test_2"), package_id: 1, - boolean_offset: 6, + boolean_offset: 3, next_offset: None, bucket_index: 0, }; @@ -265,7 +267,7 @@ mod tests { let third_node_expected = PackageTableNode { package_name: String::from("com.android.aconfig.storage.test_4"), package_id: 2, - boolean_offset: 12, + boolean_offset: 6, next_offset: None, bucket_index: 3, };