Attention is currently required from: Aaron Durbin, Furquan Shaikh, Hung-Te Lin.
5 comments:
File util/cbfstool/default.fmd:
There should be no space here, right? (I mean, I guess it may still work if there is, but I think it looks better without and that's how the documentation suggests it.)
Do we really want to not enforce alignment for CBFS? After all we do have the -a flag for cbfstool which is also regularly used for various things, and that doesn't make sense if the parent section is not aligned. I think making CBFS default to 4K as well makes sense. (Does this not build if you do that? If so, can we fix those cases?)
File util/cbfstool/fmap_from_fmd.c:
I think this should be a hard error, but I guess the plan is to make it a warning first and then change to error after some adjustment period?
File util/cbfstool/fmd_parser.y:
Patch Set #13, Line 103: | region_flag ',' region_flags { $$.v = $1.v | $3.v; };
This doesn't look wrong, but unrelated? Also, does the previous rule (`region_flag region_flags`) even make sense (would that implicitly be space-separated or does it just mean you're literally supposed to write `CBFSPRESERVE`, which then wouldn't work because the tokenizer sees it as one word)? If not, shouldn't you replace it rather than add another alternative?
File util/cbfstool/fmd_scanner.l:
Patch Set #13, Line 31: . return *yytext;
I don't really understand lex at all, but if you want to add `,` as a new valid token for separating flags, don't you need to mention it somewhere here as well? I'm confused why the percent sign goes here but the comma doesn't, since they're basically used in the same manner.
To view, visit change 37262. To unsubscribe, or for help writing mail filters, visit settings.