Minor fixes to pacify Coverity code scan #377
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Behdad,
we regularly run Coverity code scans on the OpenJDK sources and recently discovered two issues with HarfBuzz. While the discovered issues are not real errors, we think that fixing them my be nevertheless worthwile in order to increase the readability of the source code.
We just wanted to ask, if you are willing to accept these changes in the upstream HarfBuzz repository because only then it would make sense to also fix them in the OpenJDK copy of HarfBuzz.
The first issue found by Coverity is the last of the following four lines from src/hb-ot-font.cc:
From the whole context it really took me some time to understand that 'symbol' should only be set to true if 'subtable' is set from 'cmap->find_subtable (3, 0)'. Coverity reports an "assignment instead of compare" which is a false positive, but we think the could would be much more readable if changed to look as follows:
The second issue is related to the following definition in src/hb-ot-layout-gpos-table.hh:
Throughout hb-ot-layout-gpos-table.hh, '&valueFormat1' is used as if it were an array of two ValueFormat objects. While extremely unlikely, a compiler could theoretically insert padding between 'valueFormat1' and 'valueFormat2' which would make the code incorrect. We would therefore propose to simply change the previous definiton into a real array:
and change the code which uses 'valueFormat' accordingly.
Thank youand best regards,
Volker