# Introduction
The goal is to unify the integer type used for ranges. libflashrom has 2 ranges in the API: write protect (wp) ranges and layout ranges. The types for these are not consistent between wp and layout, between getters and setters, and between the API and the implementation. Additionally the types are different sized depending on the platform (ie size_t on 32 vs 64 bit).
Demo: https://review.coreboot.org/c/flashrom/+/67004
# Background / Motivation
Assertion: Flashrom only cares about fairly normal platforms (32 and 64 bit pointers).
size_t: size_t is the C type returned by sizeof. It is an unsigned integer. Throughout the C library it is used as the type for counting the length of and indexing into arrays. It is at least 16 bits. It is often the same size as uintptr_t and void* but this is not required.
uintptr_t: A C type large enough to hold a pointer.
unsigned int: At least 16 bits, probably 32 bits on all the platforms we care about.
uint32_t: Exactly 32 bit type, not required to exist (ie, flashrom won't compile on platforms where this doesn’t exist).
usize: A rust type corresponding to uintptr_t.
uintptr: A go type corresponding to uintptr_t. Go also has C.size_t.
# Flashrom Today
getsize -> size_t read/write/verify -> size_t implementation: unsigned int https://source.chromium.org/chromiumos/chromiumos/codesearch/+/cc9524d9ebf5d...
Based on the implementation using `unsigned int`, Flashrom probably only works with up to 32 bit sized flashes.
layout_add_region -> size_t, (start, end) layout_get_region -> unsigned int (start, len) impl: uint32_t (start, end) https://source.chromium.org/chromiumos/chromiumos/codesearch/+/cc9524d9ebf5d...
These types are probably fine on 32 bit platforms where they are all the same. On 64 bit you could add a region that will be truncated / wrapped without warning. The get and add using different ranges; (start, end) and (start, len) is very confusing - I have already gotten this wrong several times while refactoring.
wp_set_range -> size_t (start, len) wp_get_range -> size_t (start, len) impl: size_t (start, len)
All consistent here.
wp_ranges_count -> size_t wp_ranges_get_range -> unsigned int impl: size_t
Changing the get_range to take a size_t seems like a simple fix here. This is unlikely to ever cause a bug but is a strange style.
# Rust
Bindgen can be told to treat size_t as usize. Without this flag a rust type for size_t is chosen for the platform (ie u32 on 32 bit, u64 on 64 bit). Only usize can be used to index slices in rust.
# Proposal
## Goal
Unify all the length types (internal and external) Unify all the range bounds ((start, len) vs (start, end)) Make bindings straightforward
## Non goal 16 bit platforms
32 bit sized flashes
Change the libflashrom API and the flashrom internals to use uint32_t for all flash sizes and ranges. Keep size_t for host related sizes, eg the buffer size in flashrom_layout_read_fmap_from_buffer or flashrom_image_verify.
Add a layout_add (start, len) method and try to deprecate the (start, end) method. This will make all of the methods use the same concept of range. The wp implementation uses (start, len) while the layout implementation uses (start, end). I do not propose to change this.
In the rust fat bindings, use usize to represent all ranges and offsets. After libflashrom changes from the current implementation to uint32_t, the rust bindings must convert between u32 and usize. We will only support 32 and 64 bit usize, so this will be straightforward. The rust bindings will reject attempts to provide values > 32bit at runtime.
# Alternatives
Use uint64_t. This has the pro of supporting large SPI flash on 64 bit platforms. However this wont make large flash supported on 32 bit platforms, as single buffers are passed around for data access; flashrom_image_write (struct flashrom_flashctx *flashctx, void *buffer, size_t buffer_len, const void *refbuffer)
Use size_t everywhere. Size_t makes some sense to use, as the implementation and API deals with full flash sized buffers, and they can only be indexed by size_t sized ranges anyway. For example flashrom would support 16 bit of flash on a 16 bit platform, and 64 bit of flash on a 64 bit platform. However, tying the usable flash size to the platform size doesn't really make sense and is likely to cause confusion, and we probably only care about 32 bit flash.
Use a typedef, eg chipsize_t. I think this is reasonable internally but don't prefer it. It would theoretically allow a simple change to swap from 32 to 64 bit flashes. However C won't warn about code using the wrong type while they are 'compatible', see for example the functions using unsigned int and uint32_t interchangeably in the codebase already. And this won't really work for libflashrom, it's too messy to make the API function differently depending on library compile time choices, this really creates incompatible APIs with identical names.
For supporting larger-than-size_t size data objects, see posix lseek64 and friends.
Use u32 for the rust API. This makes sense as it pushes out-of-range errors into the user code. However it complicates a lot of usage as u32 constantly need to be converted into usize to do array indexing.