Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 646856 - flag-o-matic.eclass: clean strip-flags up
Summary: flag-o-matic.eclass: clean strip-flags up
Status: RESOLVED INVALID
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal with 1 vote (vote)
Assignee: Gentoo Toolchain Maintainers
URL: https://github.com/kerframil/gentoo/c...
Whiteboard:
Keywords: PATCH
Depends on:
Blocks:
 
Reported: 2018-02-07 02:53 UTC by kfm
Modified: 2019-12-28 13:07 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments
bug-646856-01.patch (bug-646856-01.patch,3.68 KB, patch)
2018-02-07 02:55 UTC, kfm
Details | Diff
bug-646856-02.patch (bug-646856-02.patch,1.03 KB, patch)
2018-02-07 02:56 UTC, kfm
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description kfm 2018-02-07 02:53:40 UTC
The -O2 fallback code does in strip-flags() does not work correctly because the name of the "new" array cannot be dereferenced by the _is_flagq() function. Please review and apply the attached patches, which contains far more detail on the issue, as well as a fix.

There are two patches, one of which cleans up the strip-flags function, and the other of which cleans up the _is_flagq() function. These patches may also be viewed here: https://github.com/kerframil/gentoo/commits/stable.
Comment 1 kfm 2018-02-07 02:55:37 UTC
Created attachment 518314 [details, diff]
bug-646856-01.patch

Patch for strip-flags().
Comment 2 kfm 2018-02-07 02:56:22 UTC
Created attachment 518316 [details, diff]
bug-646856-02.patch

Patch for _is_flagq().
Comment 3 Sergei Trofimovich (RETIRED) gentoo-dev 2018-02-07 07:36:39 UTC
> The -O2 fallback code does in strip-flags() does not work correctly because the name of the "new" array cannot be dereferenced by the _is_flagq() function.

What was the input CFLAGS you were testing, what was the expected result and the output?

You can try to add a test to eclass/tests/flag-o-matic.sh to get the idea about the case you are fixing.
Comment 4 kfm 2018-02-07 12:48:51 UTC
Thanks for pointing me to the formal tests, Sergei; I'd entirely forgotten that they existed. Unfortunately, it doesn't matter because I was wrong about the -O2 fallback path being non-functional! I swear that I tested my presumptions (regarding function-local scope) before posting, but my own test case was clearly incorrect and led me to an erroneous conclusion.

I knew - but forgot somewhere along the line - that bash uses dynamic scoping, whereby a variable that isn't local to the current function is attempted to be resolved in the previous function, all the way up the stack to the global scope. So, the indirect referencing in _is_flagq works for its intended use case and, technically, this bug is INVALID. Sorry about that.

That said, I still think that the existing code is hairy and does rely very heavily upon word splitting and potentially unsafe expansions that are trivially avoided (the flexibility of read under bash is a great asset). For what its worth, my code does pass all of the existing flag-o-matic tests, and is safer and faster. If anyone feels like incorporating it then great. Otherwise, c'est la vie.