On 7/4/19 2:18 AM, Oliver Browne wrote: > See below for modified patch, indentation and newline for curly braces > style applied, and commented out chunk removed. Apologies, indentation > and newline for scope are not they way I normally write things, habits > got the better of me, and I forgot to remove the commented out chunk > before submission. > > Adding the scope braces and writing the for loop in ordinary C instead > of relying on a macro are changes made for the sake of > maintainability. The right way to do this in GCC is to use the facilities we have designed for these purposes. In some cases there are constraints that those facilities enforce and doing an open-coded implementation will not work the way you want (the immediate use iterators immediately come to mind) and in other cases the facilities we've built may be much faster (the bitmap iterators) and in some cases it's just syntatic sugar, but the consistency with the rest of the codebase is important.
When you don't follow conventions, the maintainer looking at your patch has to disentangle the real change from your formatting and convention changes, then apply the fix by hand, possibly getting it wrong in the process (made even more possible by the lack of a test in the patch). THen because they made changes, they'll have to sanity tests to make sure they didn't goof anything with a typo, etc which takes even more of their limited time. If you make it easy to review the patch by following conventions, not making extraneous changes, include tests, indicate that you regression tested your patch, etc, then it becomes very easy for a maintainer to look at the patch and gate it in. Anyway, I've fixed up your patch to follow existing conventions, bootstrapped and regression tested it and installed the patch on the trunk. jeff
