Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added big endian feature for little endian CPU #697

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions c2rust-bitfields-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct BFFieldAttr {
name: String,
ty: String,
bits: (String, proc_macro2::Span),
endian: Option<String>,
}

fn parse_bitfield_attr(
Expand All @@ -31,6 +32,7 @@ fn parse_bitfield_attr(
let mut ty = None;
let mut bits = None;
let mut bits_span = None;
let mut endian = None;

if let Meta::List(meta_list) = attr.parse_meta()? {
for nested_meta in meta_list.nested {
Expand All @@ -53,6 +55,7 @@ fn parse_bitfield_attr(
bits = Some(rhs_string);
bits_span = Some(meta_name_value.path.span());
}
"endian" => endian = Some(rhs_string),
// This one shouldn't ever occur here,
// but we're handling it just to be safe
"padding" => {
Expand Down Expand Up @@ -97,6 +100,7 @@ fn parse_bitfield_attr(
name: name.unwrap(),
ty: ty.unwrap(),
bits: (bits.unwrap(), bits_span.unwrap()),
endian: endian,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
endian: endian,
endian,

}))
}

Expand Down Expand Up @@ -228,6 +232,17 @@ fn bitfield_struct_impl(struct_item: ItemStruct) -> Result<TokenStream, Error> {
let field_bit_info = field_bit_info?;
let field_bit_info_setters = &field_bit_info;
let field_bit_info_getters = &field_bit_info;
let field_endians: Vec<_> = bitfields
.iter()
.map(|field| {
field
.endian
.as_ref()
.map_or(false, |endian| endian == "big")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it only check for big or should it have to be big, little, or nothing?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endian could be an enum, and if it had a default, this could be .unwrap_or_default(). (I lack the context here to understand whether a default endianness makes sense at all; gut says no).

})
.collect();
Comment on lines +235 to +243
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let field_endians: Vec<_> = bitfields
.iter()
.map(|field| {
field
.endian
.as_ref()
.map_or(false, |endian| endian == "big")
})
.collect();
let field_endians = bitfields
.iter()
.map(|field| {
field
.endian
.as_ref()
.map_or(false, |endian| endian == "big")
})
.collect::<Vec<_>>();

let field_endians_setters = &field_endians;
let field_endians_getters = &field_endians;

// TODO: Method visibility determined by struct field visibility?
let q = quote! {
Expand All @@ -240,7 +255,7 @@ fn bitfield_struct_impl(struct_item: ItemStruct) -> Result<TokenStream, Error> {

let field = &mut self.#field_names_setters;
let (lhs_bit, rhs_bit) = #field_bit_info_setters;
int.set_field(field, (lhs_bit, rhs_bit));
int.set_field(field, (lhs_bit, rhs_bit), #field_endians_setters);
}

/// This method allows you to read from a bitfield to a value
Expand All @@ -251,7 +266,7 @@ fn bitfield_struct_impl(struct_item: ItemStruct) -> Result<TokenStream, Error> {

let field = &self.#field_names_getters;
let (lhs_bit, rhs_bit) = #field_bit_info_getters;
<IntType as FieldType>::get_field(field, (lhs_bit, rhs_bit))
<IntType as FieldType>::get_field(field, (lhs_bit, rhs_bit), #field_endians_getters)
}
)*
}
Expand Down
26 changes: 19 additions & 7 deletions c2rust-bitfields/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub trait FieldType: Sized {

fn get_bit(&self, bit: usize) -> bool;

fn set_field(&self, field: &mut [u8], bit_range: (usize, usize)) {
fn set_field(&self, field: &mut [u8], bit_range: (usize, usize), big_endian: bool) {
fn zero_bit(byte: &mut u8, n_bit: u64) {
let bit = 1 << n_bit;

Expand All @@ -28,7 +28,11 @@ pub trait FieldType: Sized {
let (lhs_bit, rhs_bit) = bit_range;

for (i, bit_index) in (lhs_bit..=rhs_bit).enumerate() {
let byte_index = bit_index / 8;
let byte_index = if big_endian {
field.len() - 1 - (bit_index / 8)
} else {
bit_index / 8
};
Comment on lines +31 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let byte_index = if big_endian {
field.len() - 1 - (bit_index / 8)
} else {
bit_index / 8
};
let byte_index = bit_index / 8;
let byte_index = if big_endian {
field.len() - 1 - byte_index
} else {
byte_index
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, can you refactor all 3 (the 2 below changes, too) of these same computations into some helper function?

let byte = &mut field[byte_index];

if self.get_bit(i) {
Expand All @@ -39,7 +43,7 @@ pub trait FieldType: Sized {
}
}

fn get_field(field: &[u8], bit_range: (usize, usize)) -> Self;
fn get_field(field: &[u8], bit_range: (usize, usize), big_endian: bool) -> Self;
}

macro_rules! impl_int {
Expand All @@ -52,12 +56,16 @@ macro_rules! impl_int {
((*self >> bit) & 1) == 1
}

fn get_field(field: &[u8], bit_range: (usize, usize)) -> Self {
fn get_field(field: &[u8], bit_range: (usize, usize), big_endian: bool) -> Self {
let (lhs_bit, rhs_bit) = bit_range;
let mut val = 0;

for (i, bit_index) in (lhs_bit..=rhs_bit).enumerate() {
let byte_index = bit_index / 8;
let byte_index = if big_endian {
field.len() - 1 - (bit_index / 8)
} else {
bit_index / 8
};
let byte = field[byte_index];
let bit = 1 << (bit_index % 8);
let read_bit = byte & bit;
Expand Down Expand Up @@ -94,12 +102,16 @@ impl FieldType for bool {
*self
}

fn get_field(field: &[u8], bit_range: (usize, usize)) -> Self {
fn get_field(field: &[u8], bit_range: (usize, usize), big_endian: bool) -> Self {
let (lhs_bit, rhs_bit) = bit_range;
let mut val = false;

for bit_index in lhs_bit..=rhs_bit {
let byte_index = bit_index / 8;
let byte_index = if big_endian {
field.len() - 1 - (bit_index / 8)
} else {
bit_index / 8
};
let byte = field[byte_index];
let bit = 1 << (bit_index % 8);
let read_bit = byte & bit;
Expand Down