pack: stop prepending a space to model, manufacturer and id

[akiernan]

pack_rkaf prepends a leading space to three fixed-width identity fields of the RKAF header:

let model_str = if model.starts_with(' ') { model.to_string() }
                else { format!(" {}", model) };

and the same for manufacturer, and unconditionally for id.

That looks like it is imitating genuine Rockchip images, which really do carry a leading space on the manufacturer field - but not because the vendor packer put one there. Comparing three RK3308 factory update images against the parameter.txt each one carries, field by field:

parameter.txt              RKAF header
MACHINE_MODEL:RK3308       model='RK3308'
MACHINE_ID:007             id='007'
MANUFACTURER: RK3308       manufacturer=' RK3308'

All three images, identical. The header holds each value exactly as the parameter file spelled it, space and all. rkpack.c writing the fields with a plain "%s", and rkflashtool's rkunpack.c reading the model straight from offset 0x08, are the same story from the other side. The vendor tool prepends nothing: the space belongs to one line of the vendor's parameter.txt, MANUFACTURER being the only one of the three the Rockchip parameter-file spec's own example writes spaced.

So this code matched the reference output on manufacturer by accident and diverged from it on the other two - backwards from following it. id is the clearest case: the value is read from parameter.txt and .trim()ed, discarding whatever spacing the file used, then a space is synthesised back on. Copying verbatim is both simpler and what the vendor does.

These are machine-readable fields. A consumer comparing model against a known board identity gets a mismatch for a reason that has nothing to do with the image, and its only ways out are to strip whitespace from a fixed-width binary field or to hard-code an offset of 9. It also spends a byte of a bounded field that write_cstr_preserving_tail already has to share with the NUL.

Nothing is taken away from callers that want the vendor's spacing: model and manufacturer are supplied by the caller and are now written through untouched, so passing " RK3308" still yields " RK3308". Where the leading space goes is the caller's decision, which is the point - it should be a decision, not something the packer does behind them.

Two existing tests asserted the space (" RK3588\0", " 007\0") - their expectations are updated. Neither was testing the padding: one covers string-field tail preservation across a repack, the other that a removed MACHINE_ID does not leak from a saved template, and both still cover exactly that. A new test pins the fixed behaviour, checking all three fields land flush and that bytes 8, 42 and 72 are not spaces.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.