Code review comment for lp:~gl-az/percona-server/ST-31919-5.1

George,

I agree with the general idea of the fix. I would try to keep the
patch minimal and avoid refactoring to simplify maintenance, but
that's more a personal preference. The only significant comment that
I have is that we seem to introduce a memory leak:

- before the patch, 'stored_result' is guaranteed to also be in the
  'results' array. Which ensures that ha_federated::reset() calls
  mysql_free_result() for all stored results. Now last_result is
  guaranteed to NOT be in the 'stored_results' array. Don't we need
  an explicit mysql_free_result() for it on reset?

Minor stuff:

- the test case needs a header referencing bug number(s)
- no need to include include/not_embedded.inc in the test case, as
  it is already included from suite/federated/include/federated.inc
- unnecessary whitespace changes on lines 411 (the server code
  formatting rules require 2 blank lines between functions), 439 and 447
- wrong comment formatting in ha_federated.h (again, server code
  formatting is different from InnoDB)