Skip to content

Allow editing empty FRU fields. #209

Closed
wants to merge 1 commit into from
Closed

Allow editing empty FRU fields. #209

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 13 additions & 4 deletions lib/ipmi_fru.c
View file
Open in desktop
Expand Up @@ -52,7 +52,6 @@
#endif

#define FRU_MULTIREC_CHUNK_SIZE (255 + sizeof(struct fru_multirec_header))
#define FRU_FIELD_VALID(a) (a && a[0])

static const char *section_id[4] = {
"Internal Use Section",
Expand Down Expand Up @@ -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);
Copy link
Author

Choose a reason for hiding this comment

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

Check the allocation before returning.

}
str = malloc(size+1);
if (!str)
Expand Down Expand Up @@ -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) {
Copy link
Author

Choose a reason for hiding this comment

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

Use FRU_END_OF_FIELDS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you talking to yourself in your own PR ? You can just push an updated version.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

printf("Field not found !\n");
rc = -1;
goto ipmi_fru_set_field_string_out;
}
fru_area = (uint8_t *) get_fru_area_str(fru_data, &fru_field_offset);
}

if (!FRU_FIELD_VALID(fru_area)) {
if (!fru_area) {
printf("Field not found !\n");
rc = -1;
goto ipmi_fru_set_field_string_out;
Expand Down Expand Up @@ -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) {
Copy link
Author

Choose a reason for hiding this comment

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

Use FRU_END_OF_FIELDS.

printf("Field not found !\n");
rc = -1;
goto ipmi_fru_set_field_string_rebuild_out;
}
fru_area = (uint8_t *) get_fru_area_str(fru_data_old, &fru_field_offset);
}

if (!FRU_FIELD_VALID(fru_area)) {
if (!fru_area) {
printf("Field not found (1)!\n");
rc = -1;
goto ipmi_fru_set_field_string_rebuild_out;
Expand Down