aconfig: add flag type in flag table and remove info byte from value
array
1, add flag type to the flag table. Before flag table only stores the
mapping from (package id, flag name) to (flag id u32). The original
intent is to do bitmasking on the top byte of flag id to indicate flag
type. Now split the flag id u32 to two u16, the first represent flag
type, the second represent flag id. So after the change, the flag table
now shows the following mapping:
(package id, flag name) -> (flag type as u16, flag id as u16)
2, originally we plan to store a info byte together with each flag
value. The info byte is used by storage service damemon to mark up the
flag status, such as if it is accepting server side flag push. After
internal discussion, it is better to just create the info bytes as
another file by storage service damemon. So that the value file is
purely a flag value array.
Bug: b/312243587
test: atest aconfig.test
Change-Id: I7f953076b4269cf786bc23723078290e5ebe10bc
diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs
index 1a8872b..f7a6417 100644
--- a/tools/aconfig/src/commands.rs
+++ b/tools/aconfig/src/commands.rs
@@ -361,7 +361,7 @@
Ok(modified_parsed_flags)
}
-pub fn assign_flag_ids<'a, I>(package: &str, parsed_flags_iter: I) -> Result<HashMap<String, u32>>
+pub fn assign_flag_ids<'a, I>(package: &str, parsed_flags_iter: I) -> Result<HashMap<String, u16>>
where
I: Iterator<Item = &'a ProtoParsedFlag> + Clone,
{
@@ -371,7 +371,13 @@
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 @@
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 46753f0..5cfe3a5 100644
--- a/tools/aconfig/src/storage/flag_table.rs
+++ b/tools/aconfig/src/storage/flag_table.rs
@@ -58,18 +58,26 @@
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<u32>,
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 @@
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
@@ -108,7 +117,11 @@
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::<Result<Vec<_>>>()
}
@@ -177,7 +190,7 @@
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 {
@@ -202,7 +215,8 @@
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),
@@ -218,13 +232,15 @@
fn new_expected(
package_id: u32,
flag_name: &str,
- flag_id: u32,
+ flag_type: u16,
+ flag_id: u16,
next_offset: Option<u32>,
bucket_index: u32,
) -> Self {
Self {
package_id,
flag_name: flag_name.to_string(),
+ flag_type,
flag_id,
next_offset,
bucket_index,
@@ -308,14 +324,17 @@
let nodes: &Vec<FlagTableNode> = &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 76835e0..a28fccd 100644
--- a/tools/aconfig/src/storage/mod.rs
+++ b/tools/aconfig/src/storage/mod.rs
@@ -56,6 +56,8 @@
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 @@
}
// 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 @@
use super::*;
use crate::Input;
+ /// Read and parse bytes as u16
+ pub fn read_u16_from_bytes(buf: &[u8], head: &mut usize) -> Result<u16> {
+ 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<u32> {
let val = u32::from_le_bytes(buf[*head..*head + 4].try_into()?);
@@ -218,13 +226,13 @@
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 1a3bbc3..0ce1349 100644
--- a/tools/aconfig/src/storage/package_table.rs
+++ b/tools/aconfig/src/storage/package_table.rs
@@ -57,6 +57,8 @@
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<u32>,
pub bucket_index: u32,
@@ -249,7 +251,7 @@
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 @@
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,
};