Allow editing empty FRU fields. #209
Conversation
This adds support for editing specifically exiting FRU fields with 0-byte length. It does not add support for creating FRU fields that do not yet exist. This allows an e.g. a system integrator to configure product FRU fields that are left empty by a board vendor.
| @@ -186,7 +185,7 @@ char * get_fru_area_str(uint8_t * data, uint32_t * offset) | |||
|
|
|||
| if (size < 1) { | |||
| *offset = off; | |||
| return NULL; | |||
| return calloc(1, 1); | |||
There was a problem hiding this comment.
Check the allocation before returning.
| @@ -4804,10 +4803,15 @@ f_type, uint8_t f_index, char *f_string) | |||
| if (fru_area) { | |||
| free_n(&fru_area); | |||
| } | |||
| if (fru_data[fru_field_offset] == 0xc1) { | |||
There was a problem hiding this comment.
Why are you talking to yourself in your own PR ? You can just push an updated version.
There was a problem hiding this comment.
Just in case somebody reads the PR before I get to pushing a new version. We're actually now looking at some failures possibly resuting from editing certain fields/offsets/lengths we found in testing, so further changes are likely needed.
There was a problem hiding this comment.
I'm not surpised. As I said here: #191 (comment), the fru code is inherently awful. In general it's a "HERE BE DRAGONS" territory. I'm not sure I will accept your PR before the rewrite of fru is done.
| @@ -4972,10 +4976,15 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId, | |||
| for (i = 0;i <= f_index; i++) { | |||
| fru_field_offset_tmp = fru_field_offset; | |||
| free_n(&fru_area); | |||
| if (fru_data_old[fru_field_offset] == 0xc1) { | |||
|
As @AlexanderAmelkin mentioned, it's probably not productive to make complex changes to this code so I'm withdrawing this pull request. We'll look into using something like frugen along with |
This adds support for editing specifically exiting FRU fields with 0-byte length. It does not add support for creating FRU fields that do not yet exist. This allows an e.g. a system integrator to configure product FRU fields that are left empty by a board vendor.
As I understand, a 0-byte field is valid per the FRU info storage definition, so inserting new data into those fields should be a valid and supported operation.
As a result, the field validity check can be simplified to a null test. If you would prefer keeping the self-documenting name of the
FRU_FIELD_VALIDmacro I can restore it, but for now I have just written the tests inline.