On 08/04/2016 01:24 PM, David Malcolm wrote:

Do you realize that this isn't used for ~700 lines after this point?
 Is
there any sensible way to factor some code here to avoid the coding
disconnect.  I realize the function was huge before you got in here,
but
if at all possible, I'd like to see a bit of cleanup.

I think this is OK after that cleanup.

format_chars can get modified in numerous places in the intervening
lines, which is why I stash the value there.
Yea, I figured that was the case. I first noticed the stashed value, but didn't see where it was used for far longer than I expected.


I can do some kind of cleanup of check_format_info_main, maybe
splitting out the things in the body of loop, moving them to support
functions.
That's essentially what I was thinking.


That said, I note that Martin's sprintf patch:
  https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00056.html
also touches those ~700 lines in check_format_info_main in over a dozen
places.  Given that, would you prefer I do the cleanup before or after
the substring_loc patch?
I think you should go first with the cleanup. It'll cause Martin some heartburn, but that happens sometimes.

And FWIW, if you hadn't needed to stash away that value I probably wouldn't have noticed how badly that function (and the loop in particular) needed some refactoring.

jeff

Reply via email to