[Commits] [wesnoth/wesnoth] a5dd65: campaignd: Use utils::lowercase() instead of a dir...

Sat Oct 11 00:39:41 UTC 2014

  Branch: refs/heads/1.12
  Home:   https://github.com/wesnoth/wesnoth
  Commit: a5dd652970cc23e09ae33afe1c427a5787d01080
      https://github.com/wesnoth/wesnoth/commit/a5dd652970cc23e09ae33afe1c427a5787d01080
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Use utils::lowercase() instead of a direct call to libc tolower()

Note that the next loop in the same chunk of code already uses
utils::lowercase() even though both strings are really of the same
nature.

(Backported 33503f2f57b757d60edee2d5d6ecb616cd4fbd78 from master.)

  Commit: 33808a29028091800dec190e5e255e6373d95b80
      https://github.com/wesnoth/wesnoth/commit/33808a29028091800dec190e5e255e6373d95b80
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    A src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Rename and refactor campaign_server class into campaignd::server

This is the first refactoring step.

This moves the class declaration to a separate header file and switches
around some implementation details. Included in this commit are a few
minor documentation changes, a lot of whitespace and placement changes,
and the following API and behavior changes:

 * Reordered server class fields, keeping net_manager_ and
   server_manager_ as the last to initialize.
 * The campaignd hooks configuration (which hasn't been used in ages) is
   now done in the load_config() method rather than directly in the
   constructor.
 * Both read-only mode configuration and control socket set-up are now
   done in load_config() rather than the run() method.

Included as well are a few Doxygen documentation changes of varying
scope. Most notably, the description for the fire() method's return
value is gone since the method was made to not return a value some time
after its inception.

I also took the liberty to spatially-reorganize some of the involved
code except for the fire() method's body (which is massive and had to be
reindented anyway), in order to make the code more fluid and clean to
read.

Overall, there are deliberately no behavior changes in this step other
than the aforementioned minor bits, in order to make the next steps
easier to plan, perform, and debug.

(Backported 23ce3f8574f8888ff84e88e81a5b105f189fc712 from master.)

Conflicts:
	src/campaign_server/campaign_server.cpp

  Commit: df6aad4502b440f331a510d34a5647d83bcca945
      https://github.com/wesnoth/wesnoth/commit/df6aad4502b440f331a510d34a5647d83bcca945
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Rename a server class field and some scoped_ostream objects

The file_ field is now cfg_file_ to avoid ambiguity ("what file and for
what?"). Since there are a lot of scoped_ostream instantiations around
named cfgfile which are used to write to the cfg_file_ path, those are
now renamed to simply out, also to avoid ambiguity -- just an
intermediate step before refactoring those into a separate method.

(Backported 796b77d44023d0fa555dcca0856d596887d89205 from master.)

  Commit: 6df2cfd9b335fee82d7243ab66a84fd982b18bb7
      https://github.com/wesnoth/wesnoth/commit/6df2cfd9b335fee82d7243ab66a84fd982b18bb7
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Refactor config commit boilerplate into a separate method

(Backported ada5a7ca60a3280044d201ad5e33c8dbaed4c56f from master.)

  Commit: 29c77cbd611bc01be41298aa66cfa82bbd35c53b
      https://github.com/wesnoth/wesnoth/commit/29c77cbd611bc01be41298aa66cfa82bbd35c53b
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Make input_ a boost::scoped_ptr

(Backported deea32f9a5fd4bb4d2b365945ebc35e2e7a90585 from master.)

  Commit: 52337f93513ec110a0848d1a74ec36ee74e6efa7
      https://github.com/wesnoth/wesnoth/commit/52337f93513ec110a0848d1a74ec36ee74e6efa7
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Replace write_config() scheduling mechanism

Check time() deltas instead of incrementing a counter variable forever.
This should allow to extract a bit of the run() logic into a separate
method later.

(Backported a8ea4796a0db6ef344da0259f2758048ac6997ce from master.)

  Commit: 8f6d82bcb7e48b98417885f80894306d20e95e89
      https://github.com/wesnoth/wesnoth/commit/8f6d82bcb7e48b98417885f80894306d20e95e89
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Refactor request handling blocks into separate methods

These methods are tried and run in sequence by looking them up from a
request handler registry maintained as part of the server object. Since
every campaignd request is a single WML node with an identifying name, I
feel this approach makes things more readable than the previous massive
deeply-nested if-else-if chain approach, but that might be just me.

(Backported c164cb22b04601a1329274ab65934d02a83ec143 from master.)

Conflicts:
	src/campaign_server/campaign_server.cpp

  Commit: 04c97adbb23b15cb546f1aa7d0f8fe1c6d1ab161
      https://github.com/wesnoth/wesnoth/commit/04c97adbb23b15cb546f1aa7d0f8fe1c6d1ab161
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Refactor construct_message/error() functions and boilerplate

This replaces them with two methods that are part of the server class
itself.

These functions were always used in conjunction with
network::send_data() to send the generated WML object to a network
client. Because the config was created within the functions and not on
the callsite, this resulted in a call to the config copy constructor
that could be avoided by grouping the network::send_data() call together
with the WML generation.

(Backported e5354435b185dd4e4b02823de972d0630c3af59d from master.)

Conflicts:
	src/campaign_server/campaign_server.cpp

  Commit: 8a65dfc8597dd2a85174167006a892d5054f1b29
      https://github.com/wesnoth/wesnoth/commit/8a65dfc8597dd2a85174167006a892d5054f1b29
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/CMakeLists.txt
    M src/SConscript
    A src/campaign_server/addon_utils.cpp
    A src/campaign_server/addon_utils.hpp
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Move several anonymous namespace members to a new file

This step covers the markup char check function and the add-on feedback
URL formatter.

(Backported 0d81fae45ba767a8127cec896f9c0f5d16d1bb82 from master.)

  Commit: 25846d0bbc1d195009c4d1dfa14c63597e282439
      https://github.com/wesnoth/wesnoth/commit/25846d0bbc1d195009c4d1dfa14c63597e282439
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/addon_utils.cpp
    M src/campaign_server/addon_utils.hpp
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Move and document more ancillary functions

Since the move is between two files, I took this opportunity to rename
a "campaign" parameter to "addon" and tweak the code style for
consistency.

(Backported 42ee57c91934fff7f5920ce56eb413dff84963a8 from master.)

  Commit: b38e4775cbc370afcf50faa0f4dff6aad1fde0f2
      https://github.com/wesnoth/wesnoth/commit/b38e4775cbc370afcf50faa0f4dff6aad1fde0f2
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Wrap request handler registration statements in macros

This should make it easier to add new handlers or change their API at a
later point.

(Backported 7535b916a3f03191bba3cf48b8d97a64e5930031 from master.)

  Commit: 47c9b63f9c8b386361d41cecbebdcdd334a9222d
      https://github.com/wesnoth/wesnoth/commit/47c9b63f9c8b386361d41cecbebdcdd334a9222d
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Drop pointless use of lexical_cast to set WML attributes

Since 1.9.x, using lexical_cast to set config attributes is no longer
required and the engine will automatically try to cast string values to
a more "natural" type.

Thus, this commit incurs in no real behavior changes.

(Backported dff480bb0765beb34ed9192c5e2ca284ed522a2c from master.)

  Commit: 7f39a46e90c991816f24c9415709dfcf987dad58
      https://github.com/wesnoth/wesnoth/commit/7f39a46e90c991816f24c9415709dfcf987dad58
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Use empty C strings instead of t_strings to clear out attributes

(Backported 9b1fd86016d09fdc9d4999d04a7bff38cdaeec05 from master.)

  Commit: 5b2ae9c9582d44bb8e03d0491b2358544cb737f7
      https://github.com/wesnoth/wesnoth/commit/5b2ae9c9582d44bb8e03d0491b2358544cb737f7
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Show RO mode warning in load_config() rather than in run()

(Backported 603c06d1e7793f62e201df09dd5bfe3c46e84c5b from master.)

  Commit: d9b023e019322805189e56652d64ed6f507b2795
      https://github.com/wesnoth/wesnoth/commit/d9b023e019322805189e56652d64ed6f507b2795
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Code indentation and spacing style changes for consistency

No functional changes.

(Backported e13570954a1abfec49df6d263cbe5b69130bc43f from master.)

Conflicts:
	src/campaign_server/campaign_server.cpp

  Commit: ec943244caede3d76fa5744a8b1d90b2ae17700d
      https://github.com/wesnoth/wesnoth/commit/ec943244caede3d76fa5744a8b1d90b2ae17700d
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Use const references or values where applicable

Some old code used copies or read-write refererences for strings that
are never deliberately modified afterwards; and a bit of my own code
gets a (arguably tiny) WML config by value from a method that always
returns a read-only reference, missing out on a tiny optimization
opportunity by avoiding a config copy.

There was also a once-written int variable in handle_request_campaign().

(Backported fb5c5abeffc853665d68495085bdaf2014f6da13 from master.)

  Commit: 69520c44367e0ba373d5a61857b53853265fd01b
      https://github.com/wesnoth/wesnoth/commit/69520c44367e0ba373d5a61857b53853265fd01b
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Write config to disk every ten minutes, not 60 seconds

The 60 seconds value was just intended for internal testing and it
slipped into production by accident.

(Backported 6558bdc785bb31eae0973ff9e1c246c06ea9db0d from master.)

  Commit: 9e1d1be45e33db5ac4c30feed1749286543bea94
      https://github.com/wesnoth/wesnoth/commit/9e1d1be45e33db5ac4c30feed1749286543bea94
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Don't use GUI1 markup in [upload] response

Wesnoth 1.7.x and later use a GUI2 code path to display the [upload]
response message, which means GUI1 markup won't work with it. They do
not enable Pango markup either, and in any case, formatting messages
properly should be the client's responsibility, not the server's.

Perhaps later we should allow including a secondary message in the
response or something like that.

(Backported 610070e4e4a8d45bfeccacf518aa5a450f00ffa3 from master.)

  Commit: 37b531b839d0c16366ee58496a4d2ec8da4d84b2
      https://github.com/wesnoth/wesnoth/commit/37b531b839d0c16366ee58496a4d2ec8da4d84b2
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: A few style consistency fixes I missed before

Spacing, unary & positioning, and static functions vs. anonymous
namespace functions.

(Backported e057cd3b817a07e4c3b845749098c56e4e6fc485 from master.)

  Commit: 0b4e42a312f5b7f55bfb29d8f18a6076802f7dbc
      https://github.com/wesnoth/wesnoth/commit/0b4e42a312f5b7f55bfb29d8f18a6076802f7dbc
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Replace vector<pair<k,v>> with map<k,v> for the handlers table

std::vector<std::pair<k,v>> has worse look-up performance than
std::map<k,v> in general, not that the difference is too noticeable for
such a small dataset like campaignd's handlers table, that is built in
such a way its items are arranged in descending order of usage
frequency.

It still feels like this is the best thing to do to keep future
maintainers sane.

(Backported 65570f5cb9e92f73a96d76c48654511dc1860b00 from master.)

  Commit: 61617164bc0a6b89e7c92995f46c2c5b5f3141d5
      https://github.com/wesnoth/wesnoth/commit/61617164bc0a6b89e7c92995f46c2c5b5f3141d5
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Print the destination IP in the send_error() call

(Backported 6c5e9e37e06d9e216ce2a8bd7040c2ca06cd5943 from master.)

  Commit: b11bce16ae0aaa8f3911b1c4cf99f9ddb28bd2e4
      https://github.com/wesnoth/wesnoth/commit/b11bce16ae0aaa8f3911b1c4cf99f9ddb28bd2e4
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Add a typedef for the handlers map

(Backported 6b8e42dfe277846ef6fadd03469de0552bd15ec6 from master.)

  Commit: e6ca14f50d0356c5c1567786a25a21197b068671
      https://github.com/wesnoth/wesnoth/commit/e6ca14f50d0356c5c1567786a25a21197b068671
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Use find() to locate a request handler

Makes it so for each request received we use
request_handlers_table::find() instead of a plain linear search to find
the handler, using the tag name of the first WML child node we can find,
instead of trying to locate one with a known handler id.

In theory this allows taking advantage of the request_handlers_table
type's (currently a std::map) own lookup mechanism.

(Backported df54918c362998fce482fd6b72f5739d9a774215 from master.)

  Commit: f1dc59f653a03f146c6522436d1a9f80d1a0304e
      https://github.com/wesnoth/wesnoth/commit/f1dc59f653a03f146c6522436d1a9f80d1a0304e
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Reject bad requests with an [error] block

This can be used to signal campaignd clients in the future about
features that aren't yet implemented.

As with the previous change to using find() to locate a request handler,
this only applies to the first WML child of the request's root node.
Other nodes continue to be ignored.

(Backported efcf8f7b6aa4e25c0ed5cbd80597e2d894658d95 from master.)

  Commit: cf60d5c249bb4b4b7b805e064016874372007a2f
      https://github.com/wesnoth/wesnoth/commit/cf60d5c249bb4b4b7b805e064016874372007a2f
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  campaignd: Document send_message/send_error's WML output and side-effects

(Backported 46815ee0084101d28dbdea335680a13384945dad from master.)

  Commit: e34593b09aa59e901ace2b0c583d6cd431e1fc74
      https://github.com/wesnoth/wesnoth/commit/e34593b09aa59e901ace2b0c583d6cd431e1fc74
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Log errno to stderr if deleting an add-on archive failed

Unhandled remove() return value issue found by coverity.

This is about the most we can do about this here since internal server
errors like this shouldn't matter to clients (remove() failing is only
an issue if write_config() somehow fails or the same add-on is
reuploaded later).

Note to self: sanitize campaignd logging later so we can actually
distinguish between informational and error messages without including
the word "error" or "failure" and variations thereof in each line.

(Backported 786789b2d788d9166acc54e4bcab8f308e303a4a from master.)

  Commit: 5d744f65a66974dec771fd288a7326da3a1b1800
      https://github.com/wesnoth/wesnoth/commit/5d744f65a66974dec771fd288a7326da3a1b1800
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Catch boost::bad_function_call exception

Isssue found by Coverity.

This exception is thrown if an empty boost::function object is called.
Theoretically should never happen unless the request handlers table
wasn't initialized correctly/was tampered with.

(Backported 2ac434dd2caa2214f994a32d0555fb4ab82c1e27 from master.)

  Commit: 01338720e515a40eedad12b4ba95488ffb09281c
      https://github.com/wesnoth/wesnoth/commit/01338720e515a40eedad12b4ba95488ffb09281c
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Log messages to a new log domain of our own

Until now, campaignd logged its diagnostics to the network log domain
with the show_names parameter (of logger::operator()()) set to false so
as to avoid showing the log level or domain names in stderr. There are
two issues with this approach:

 * In order to tell the difference between informational and error
   messages, the messages themselves need to include a severity label in
   some form, which defeats half the point of using our standard logging
   facilities (though not the other half, which is having timestamps).
 * In the event that messages from other domains appear in stderr (which
   can only happen if their severity is 'warning' or more at this time,
   since campaignd doesn't change the default severity configuration),
   the log could become a jumbled mess.

Note that wesnothd (which produces a larger volume of diagnostics all
the time) does already use a log domain of its own with no overidden
show_names option.

Giving campaignd its own log domain should allow us to add more optional
diagnostics later if needed, and right now it allows us to do away with
the unwieldy severity labels in the log messages themselves in favor of
choosing the right logger in the invocation -- although this last point
is beyond the scope of this introductory commit.

This commit keeps our previous logging macro, LOG_CS, just replacing the
log domain and severity (info instead of err) and dropping the
overridden show_names value. Later I'll take care of choosing more
relevant severities for different messages. For now, we do like wesnothd
and select the info severity as the default during initialization so the
messages can still display.

(Backported 8f5dc70de16348e7ad45211320bff702b91ddf38 from master.)

  Commit: 09e4374d7364752dd9bf10cecd7ff3c7f3bc5e28
      https://github.com/wesnoth/wesnoth/commit/09e4374d7364752dd9bf10cecd7ff3c7f3bc5e28
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/campaign_server/campaign_server.cpp

  Log Message:
  -----------
  campaignd: Move error messages to the error logger, drop inline "ERROR" label

This includes messages emitted by the send_error() method, which is used
all over the place.

(Backported d1dd66dcbeef346128e108b9743b5f29ff30bb82 from master.)

  Commit: 45f15e6b08ab0ada9cbf5ee470868f5a5114fa82
      https://github.com/wesnoth/wesnoth/commit/45f15e6b08ab0ada9cbf5ee470868f5a5114fa82
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-10-10 (Fri, 10 Oct 2014)

  Changed paths:
    M src/CMakeLists.txt
    M src/SConscript
    A src/campaign_server/addon_utils.cpp
    A src/campaign_server/addon_utils.hpp
    M src/campaign_server/campaign_server.cpp
    A src/campaign_server/campaign_server.hpp

  Log Message:
  -----------
  Merge branch 'backport/1.12/campaignd-refactoring' into 1.12

Compare: https://github.com/wesnoth/wesnoth/compare/74fada065e49...45f15e6b08ab