Reapply D5346: Fix uninformative hp2ps error when the command args contains double quotes with fix incompatible shell quoting in tests. It seems
like $'string' is not recognized under all test environments, so let's
avoid it in tests.
Details
hp2ps: "T15904".hp, line 2: integer must follow identifier
use new ghc and hp2ps to profile a simple program.
Diff Detail
- Repository
- rGHC Glasgow Haskell Compiler
- Branch
- hp2ps_job_parse (branched from master)
- Lint
Lint Warnings Excuse: . Severity Location Code Message Warning utils/hp2ps/HpFile.c:183 TXT3 Line Too Long Warning utils/hp2ps/HpFile.c:194 TXT3 Line Too Long - Unit
No Unit Test Coverage - Build Status
utils/hp2ps/HpFile.c | ||
---|---|---|
18 | It would be lovely if those whitespace changes could be kept separate from the actual functional changes; makes it a lot easier on reviewers, and also helps figure out merge conflicts down the road. | |
410 | This moves the allocation before the ASSERT; this is potentially wasteful (though only cosmetically so), because when the assert doesn't hold, the memory will still have been allocated. | |
416 | I found the old loop logic easier to follow than this. Either way, you have two variables that go into effective loop conditions (ch and i); the old version had ch as the principal loop condition (signalling the intent "loop until you find a double quote") and tracks i as a counter on the side; the new version has i as the principal loop condition (signalling the intent "keep incrementing a size counter forever"), and then side-aborts the loop based ch. I would say that the former is closer to the actual intent here, and the latter only adds confusion. Is there a strong, compelling reason to write it like this? |
rebase onto D5407: cosmetic change: expandtab in utils/hp2ps/HpFile.c and move ASSERT as @tdammers suggested
Looks good to me.
utils/hp2ps/HpFile.c | ||
---|---|---|
410 | I'm a bit confused: if the ASSERT fails then the program dies. Surely we don't care about an extra allocation in an unexpected failure path. |