fexc fixes #49
fexc fixes #49
Conversation
A test case for this is "fex2bin a-star_kv49l.fex". This patch fixes an execution path in script_fex.c that would allow script_parse_fex() to fail (i.e. return 0) without providing any feedback to the user. Signed-off-by: Bernhard Nortmann <[email protected]>
| @@ -271,7 +271,7 @@ static int decompile_section(void *bin, size_t bin_size, | |||
| } else if (gpio->port < 1 || gpio->port > GPIO_BANK_MAX) { | |||
| pr_err("%s: %s.%s: unknown GPIO port bank %c (%u)\n", | |||
| filename, section->name, entry->name, | |||
| 'A'+gpio->port, gpio->port); | |||
| '@'+gpio->port, gpio->port); | |||
There was a problem hiding this comment.
The '@' character looks a little bit odd here. Maybe it's better to have a cleaner error message variant for the cases when the bank number can't be mapped to a valid latin letter?
There was a problem hiding this comment.
It looks odd, but the message also prints the decimal bank number - so it should be possible for the user to identify the problem even when gpio->port <= 0. There are other places where something like 'A' + gpio->port - 1 gets used - but it don't really consider that "cleaner".
|
The "fexc: Don't fail silently on parse error" patch looks good to me. BTW, what are we going to do with the discovered broken FEX files in the sunxi-boards repository? |
No idea. 😄 I have created corresponding issues, but it remains to be seen whether anybody cares... Who's "in charge" of the sunxi-boards repo? I noticed that there also seem to be several pull requests pending. One of the reasons I'm aiming at some automated testing of the .fex files is to have a proper functional test of |
|
If that was more of a "technical" question instead of an administrative one: For now I have simply resorted to using a set of patches to the .fex files for my testing - see n1tehawk@2876aff. With those applied (and the |
Not sure about this, maybe @jwrdegoede knows something?
Yes, enabling continuous integration for sunxi-boards and doing at least some basic automatic validation for the contributed FEX files would make sense. |
|
Hi, On 24-05-16 11:32, Siarhei Siamashka wrote:
No one really is, a bunch of us has access and we all pull in patches posted to Regards, Hans |
|
Hi Hans!
Some of which are untested / broken, i.e. won't compile with our current @ssvb Siarhei - if I'm not mistaken, you have write access to the sunxi-boards repo, too. Regards, NiteHawk |
Both the error output in function decompile_section() and the
definition of GPIO_BANK_MAX in script.h suffered from an "off
by one" logic. The bank numbering in the .bin is based on 1,
so it should be added to ('A' - 1) for human-readable output,
and the maximum number corresponding to 'N' is 14.
This became apparent when trying to translate a fex2bin-compiled
a80_optimus_board.bin back to its original .fex equivalent.
Signed-off-by: Bernhard Nortmann <[email protected]>
n1tehawk
force-pushed
the
20160518_fexc-fixes
branch
from
5bb213f to
6271d37
Compare
|
I have reworked the second commit not to output any 'invalid' bank letters any more (and just use the number instead). From my point of view, this should be good to merge now? |
|
Thanks, looks good now. |
|
I'll probably add to this PR to address a quirk detected with decompiling (malformed) |
Recent original .bin files showed up with a version[0] of 49152 = 0xC000, which won't pass our current sanity checks. Restrict version[0] check to lower 14 bits, to make it pass again. (This might require further investigation in the future, as it's possible that this particular header field actually isn't related to version data at all.) Fixes linux-sunxi#51. Signed-off-by: Bernhard Nortmann <[email protected]>
script_bin.c decompiles section entries even if they have invalid indentifiers. Thus malformed .bin files were observed to result in a .fex that couldn't be processed successfully with the sunxi-fexc compiler. This patch makes bin2fex now issue a warning for this kind of keys, so that they're easier to notice and correct. Signed-off-by: Bernhard Nortmann <[email protected]>
Such lines do not conform to any known syntax rules. With this patch, fexc will assume that they represent a special case (where bin2fex extracted a malformed indentifier from a .bin file - likely a remnant from a comment typo), issue a warning and ignore the line. Signed-off-by: Bernhard Nortmann <[email protected]>
|
In the end I decided to follow a two-fold strategy here: 1. Have This should solve the problems that linux-sunxi/sunxi-boards#50 related to. Siarhei, if you're okay with the three new commits above, I'd like to merge this - allowing me to move forward with another branch adding actual .fex ↔ .bin tests to Travis CI. |
|
Thanks, this seems to be reasonable and |
|
Merged. Thanks for reviewing! |
I'm currently experimenting with functional tests that do a "
fex2bin+bin2fexcycle" on all FEX files from sunxi-boards. In doing that, two minor problems with the current code showed up. This pull request addresses those.