[XL] Change in coreboot[main]: tests: Add unit tests for CFR

[Jakub "Kuba" Czapiga (Code Review)]

Attention is currently required from: Julius Werner, Patrick Rudolph.

Jakub "Kuba" Czapiga has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/78506?usp=email )

Change subject: tests: Add unit tests for CFR
......................................................................

Patch Set 5:

(4 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/78506/comment/22f28d21_5e56d0b6?usp... :
PS5, Line 7: tests: Add unit tests for CFR
This CL does not only add unit-tests but also the base implementation. I think this commit message should reflect that. :)

File src/commonlib/bsd/cfr_parser.c:

https://review.coreboot.org/c/coreboot/+/78506/comment/4b3eb92d_d213ca01?usp... :
PS5, Line 21: case CFR_TAG_OPTION_ENUM:
            : 		return sizeof(struct lb_cfr_numeric_option);
            : 	case CFR_TAG_OPTION_NUMBER:
            : 		return sizeof(struct lb_cfr_numeric_option);
            : 	case CFR_TAG_OPTION_BOOL:
            : 		return sizeof(struct lb_cfr_numeric_option);
Is there a reason for these case statements to be separate? They can look like that:
```
    case CFR_TAG_OPTION_ENUM:
    case CFR_TAG_OPTION_NUMBER:
    case CFR_TAG_OPTION_BOOL:
    	return sizeof(struct lb_cfr_numeric_option);
```

https://review.coreboot.org/c/coreboot/+/78506/comment/b64f309c_d8684dad?usp... :
PS5, Line 109: ret = cfr_iterate(&child, callback, priv);
How deep can recursion be here? If someone passes a long enough parent->child->child->... chain then it can cause stack overflow. I think this should be limited to a safe value.

@[e-mail] WDYT?

https://review.coreboot.org/c/coreboot/+/78506/comment/0b325106_b79d1d52?usp... :
PS5, Line 144: // FIXME checksum
Sorry, but I cannot allow the input parsing code to be submitted w/o checksum checking if there is one. Please implement this feature.