You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by James Peach <jp...@apache.org> on 2014/03/04 17:36:20 UTC
Re: [1/2] git commit: TS-2522 Better hook management, and cleanup
On Mar 4, 2014, at 12:19 AM, zwoop@apache.org wrote:
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 17f680231 -> 4f0b0b447
>
>
> TS-2522 Better hook management, and cleanup
>
> This also does
>
> - Add support for system wide config directory as default
> - Better debug messages (textual hook names etc.)
> - Less overhead on creating TXN hooks
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/31792d2a
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/31792d2a
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/31792d2a
>
> Branch: refs/heads/master
> Commit: 31792d2aae9519e0feb66ab9e80b1a9d5d51d7de
> Parents: 17f6802
> Author: Leif Hedstrom <zw...@apache.org>
> Authored: Thu Feb 27 19:55:53 2014 -0700
> Committer: Leif Hedstrom <zw...@apache.org>
> Committed: Tue Mar 4 01:13:59 2014 -0700
>
> ----------------------------------------------------------------------
> CHANGES | 3 +
> plugins/header_rewrite/header_rewrite.cc | 216 ++++++++++++++++----------
> plugins/header_rewrite/lulu.h | 4 +-
> plugins/header_rewrite/operators.cc | 8 +-
> plugins/header_rewrite/parser.cc | 2 +-
> plugins/header_rewrite/resources.cc | 7 +-
> plugins/header_rewrite/resources.h | 1 -
> plugins/header_rewrite/ruleset.cc | 4 +-
> 8 files changed, 152 insertions(+), 93 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 3a83537..363ab16 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 5.0.0
>
> + *) [TS-2522] Better hook management for header_rewrite plugin, and some
> + cleanup.
> +
> *) [TS-2169] Add SSL statistics
> Author: Ron Barber <rb...@yahoo-inc.com>
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/header_rewrite.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc
> index 6fc7960..614d287 100644
> --- a/plugins/header_rewrite/header_rewrite.cc
> +++ b/plugins/header_rewrite/header_rewrite.cc
> @@ -27,40 +27,83 @@
> #include "resources.h"
>
> // Debugs
> -const char* PLUGIN_NAME = "header_rewrite";
> -const char* PLUGIN_NAME_DBG = "header_rewrite_dbg";
> +const char PLUGIN_NAME[] = "header_rewrite";
> +const char PLUGIN_NAME_DBG[] = "dbg_header_rewrite";
> +
> +const char* HOOK_NAMES[] = {
You should do some kind of static assert to force a build break if new hooks are added.
> + "TS_HTTP_READ_REQUEST_HDR_HOOK",
> + "TS_HTTP_OS_DNS_HOOK",
> + "TS_HTTP_SEND_REQUEST_HDR_HOOK",
> + "TS_HTTP_READ_CACHE_HDR_HOOK",
> + "TS_HTTP_READ_RESPONSE_HDR_HOOK",
> + "TS_HTTP_SEND_RESPONSE_HDR_HOOK",
> + "TS_HTTP_REQUEST_TRANSFORM_HOOK",
> + "TS_HTTP_RESPONSE_TRANSFORM_HOOK",
> + "TS_HTTP_SELECT_ALT_HOOK",
> + "TS_HTTP_TXN_START_HOOK",
> + "TS_HTTP_TXN_CLOSE_HOOK",
> + "TS_HTTP_SSN_START_HOOK",
> + "TS_HTTP_SSN_CLOSE_HOOK",
> + "TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK",
> + "TS_HTTP_PRE_REMAP_HOOK",
> + "TS_HTTP_POST_REMAP_HOOK",
> + "TS_HTTP_RESPONSE_CLIENT_HOOK",
> + "TS_HTTP_LAST_HOOK"
> +};
> +
> +
> +// Forward declaration for the main continuation.
> +static int cont_rewrite_headers(TSCont, TSEvent, void *);
>
> -static const char* DEFAULT_CONF_PATH = "/usr/local/etc/header_rewrite/";
>
> // Simple wrapper around a configuration file / set. This is useful such that
> // we can reuse most of the code for both global and per-remap rule sets.
> -struct RulesConfig
> +class RulesConfig
> {
> +public:
> RulesConfig()
> {
> - memset(rules, 0, sizeof(rules));
> - memset(resids, 0, sizeof(resids));
> + memset(_rules, 0, sizeof(_rules));
> + memset(_resids, 0, sizeof(_resids));
> +
> + _cont = TSContCreate(cont_rewrite_headers, NULL);
> + TSContDataSet(_cont, static_cast<void*>(this));
> }
>
> ~RulesConfig()
> {
> - for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i)
> - delete rules[i];
> + for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> + delete _rules[i];
> + }
> +
> + TSContDestroy(_cont);
> }
>
> - RuleSet* rules[TS_HTTP_LAST_HOOK+1];
> - ResourceIDs resids[TS_HTTP_LAST_HOOK+1];
> + TSCont continuation() const { return _cont; }
> +
> + ResourceIDs resid(int hook) const { return _resids[hook]; }
> + RuleSet* rule(int hook) const { return _rules[hook]; }
> +
> + bool parse_config(const std::string fname, TSHttpHookID default_hook);
> +
> +private:
> + bool add_rule(RuleSet* rule);
> +
> + TSCont _cont;
> + RuleSet* _rules[TS_HTTP_LAST_HOOK+1];
> + ResourceIDs _resids[TS_HTTP_LAST_HOOK+1];
> };
>
> // Helper function to add a rule to the rulesets
> -static bool
> -add_rule(RuleSet* rule, RulesConfig *conf) {
> +bool
> +RulesConfig::add_rule(RuleSet* rule)
> +{
> if (rule && rule->has_operator()) {
> - TSDebug(PLUGIN_NAME, "Adding rule to hook=%d\n", rule->get_hook());
> - if (NULL == conf->rules[rule->get_hook()]) {
> - conf->rules[rule->get_hook()] = rule;
> + TSDebug(PLUGIN_NAME_DBG, " Adding rule to hook=%s\n", HOOK_NAMES[rule->get_hook()]);
> + if (NULL == _rules[rule->get_hook()]) {
> + _rules[rule->get_hook()] = rule;
> } else {
> - conf->rules[rule->get_hook()]->append(rule);
> + _rules[rule->get_hook()]->append(rule);
> }
> return true;
> }
> @@ -76,17 +119,24 @@ add_rule(RuleSet* rule, RulesConfig *conf) {
> // anyways (or reload for remap.config), so not really in the critical path.
> //
> bool
> -parse_config(const std::string fname, TSHttpHookID default_hook, RulesConfig *conf)
> +RulesConfig::parse_config(const std::string fname, TSHttpHookID default_hook)
> {
> RuleSet* rule = NULL;
> - std::string filename = fname;
> + std::string filename;
> std::ifstream f;
> int lineno = 0;
>
> - // Try appending the default conf path if the fname doesn't exist.
> - if (0 != access(filename.c_str(), R_OK)) {
> - filename = DEFAULT_CONF_PATH;
> - filename += fname;
> + if (0 == fname.size()) {
> + TSError("%s: no config filename provided", PLUGIN_NAME);
> + return false;
> +
> + }
> +
> + if (fname[0] != '/') {
> + filename = TSConfigDirGet();
> + filename += "/" + fname;
> + } else {
> + filename = fname;
> }
>
> f.open(filename.c_str(), std::ios::in);
> @@ -95,8 +145,6 @@ parse_config(const std::string fname, TSHttpHookID default_hook, RulesConfig *co
> return false;
> }
>
> - TSDebug(PLUGIN_NAME, "Loading header_rewrite config from %s", filename.c_str());
> -
> while (!f.eof()) {
> std::string line;
>
> @@ -105,16 +153,19 @@ parse_config(const std::string fname, TSHttpHookID default_hook, RulesConfig *co
> TSDebug(PLUGIN_NAME_DBG, "Reading line: %d: %s", lineno, line.c_str());
>
> boost::trim(line);
> - if (line.empty() || (line[0] == '#'))
> + if (line.empty() || (line[0] == '#')) {
> continue;
> + }
>
> Parser p(line); // Tokenize and parse this line
> - if (p.empty())
> + if (p.empty()) {
> continue;
> + }
>
> // If we are at the beginning of a new condition, save away the previous rule (but only if it has operators).
> - if (p.is_cond() && add_rule(rule, conf))
> + if (p.is_cond() && add_rule(rule)) {
> rule = NULL;
> + }
>
> if (NULL == rule) {
> rule = new RuleSet();
> @@ -151,12 +202,12 @@ parse_config(const std::string fname, TSHttpHookID default_hook, RulesConfig *co
> }
>
> // Add the last rule (possibly the only rule)
> - add_rule(rule, conf);
> + add_rule(rule);
>
> // Collect all resource IDs that we need
> for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> - if (conf->rules[i]) {
> - conf->resids[i] = conf->rules[i]->get_all_resource_ids();
> + if (_rules[i]) {
> + _resids[i] = _rules[i]->get_all_resource_ids();
> }
> }
>
> @@ -174,7 +225,6 @@ cont_rewrite_headers(TSCont contp, TSEvent event, void *edata)
> TSHttpHookID hook = TS_HTTP_LAST_HOOK;
> RulesConfig* conf = static_cast<RulesConfig*>(TSContDataGet(contp));
>
> - // Get the resources necessary to process this event
> switch (event) {
> case TS_EVENT_HTTP_READ_RESPONSE_HDR:
> hook = TS_HTTP_READ_RESPONSE_HDR_HOOK;
> @@ -198,10 +248,11 @@ cont_rewrite_headers(TSCont contp, TSEvent event, void *edata)
> }
>
> if (hook != TS_HTTP_LAST_HOOK) {
> - const RuleSet* rule = conf->rules[hook];
> + const RuleSet* rule = conf->rule(hook);
> Resources res(txnp, contp);
>
> - res.gather(conf->resids[hook], hook);
> + // Get the resources necessary to process this event
> + res.gather(conf->resid(hook), hook);
>
> // Evaluation of all rules. This code is sort of duplicate in DoRemap as well.
> while (rule) {
> @@ -236,7 +287,6 @@ TSPluginInit(int argc, const char *argv[])
> if (TS_SUCCESS != TSPluginRegister(TS_SDK_VERSION_3_0 , &info)) {
> TSError("%s: plugin registration failed.\n", PLUGIN_NAME);
> }
> - TSDebug(PLUGIN_NAME, "number of arguments: %d", argc);
>
> // Parse the global config file(s). All rules are just appended
> // to the "global" Rules configuration.
> @@ -246,10 +296,12 @@ TSPluginInit(int argc, const char *argv[])
> for (int i=1; i < argc; ++i) {
> // Parse the config file(s). Note that multiple config files are
> // just appended to the configurations.
> - if (!parse_config(argv[i], TS_HTTP_READ_RESPONSE_HDR_HOOK, conf)) {
> - TSError("header_rewrite: failed to parse configuration file %s", argv[argc]);
> - } else {
> + TSDebug(PLUGIN_NAME, "Loading global configuration file %s", argv[i]);
> + if (conf->parse_config(argv[i], TS_HTTP_READ_RESPONSE_HDR_HOOK)) {
> + TSDebug(PLUGIN_NAME, "Succesfully loaded global config file %s", argv[i]);
> got_config = true;
> + } else {
> + TSError("header_rewrite: failed to parse configuration file %s", argv[i]);
> }
> }
>
> @@ -258,8 +310,8 @@ TSPluginInit(int argc, const char *argv[])
> TSContDataSet(contp, conf);
>
> for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> - if (conf->rules[i]) {
> - TSDebug(PLUGIN_NAME, "adding hook: %d", i);
> + if (conf->rule(i)) {
> + TSDebug(PLUGIN_NAME, "Adding global ruleset to hook=%s", HOOK_NAMES[i]);
> TSHttpHookAdd(static_cast<TSHttpHookID>(i), contp);
> }
> }
> @@ -293,7 +345,7 @@ TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
> return TS_ERROR;
> }
>
> - TSDebug(PLUGIN_NAME, "remap plugin is successfully initialized");
> + TSDebug(PLUGIN_NAME, "Remap plugin is successfully initialized");
> return TS_SUCCESS;
> }
>
> @@ -301,7 +353,7 @@ TSRemapInit(TSRemapInterface *api_info, char *errbuf, int errbuf_size)
> TSReturnCode
> TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSED */, int /* errbuf_size ATS_UNUSED */)
> {
> - TSDebug(PLUGIN_NAME, "initializing the remap plugin header_rewrite");
> + TSDebug(PLUGIN_NAME, "Instantiating a new remap.config plugin rule");
>
> if (argc < 3) {
> TSError("%s: Unable to create remap instance, need config file", PLUGIN_NAME);
> @@ -311,14 +363,25 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
> RulesConfig* conf = new RulesConfig;
>
> for (int i=2; i < argc; ++i) {
> - if (!parse_config(argv[i], TS_REMAP_PSEUDO_HOOK, conf)) {
> + TSDebug(PLUGIN_NAME, "Loading remap configuration file %s", argv[i]);
> + if (!conf->parse_config(argv[i], TS_REMAP_PSEUDO_HOOK)) {
> TSError("%s: Unable to create remap instance", PLUGIN_NAME);
> return TS_ERROR;
> + } else {
> + TSDebug(PLUGIN_NAME, "Succesfully loaded remap config file %s", argv[i]);
> }
> }
>
> - *ih = conf;
> - TSDebug(PLUGIN_NAME, "added header_rewrite remap rule set");
> + // For debugging only
> + if (TSIsDebugTagSet(PLUGIN_NAME)) {
> + for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> + if (conf->rule(i)) {
> + TSDebug(PLUGIN_NAME, "Adding remap ruleset to hook=%s", HOOK_NAMES[i]);
> + }
> + }
> + }
> +
> + *ih = static_cast<void*>(conf);
>
> return TS_SUCCESS;
> }
> @@ -338,57 +401,46 @@ TSRemapDeleteInstance(void *ih)
> TSRemapStatus
> TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
> {
> - TSRemapStatus rval = TSREMAP_NO_REMAP;
> -
> + // Make sure things are properly setup (this should never happen)
> if (NULL == ih) {
> TSDebug(PLUGIN_NAME, "No Rules configured, falling back to default");
> - return rval;
> - } else {
> - RulesConfig* conf = static_cast<RulesConfig*>(ih);
> -
> - // TODO: We should optimize this to not create continuations every time,
> - // but instead save the continuations, with configurations, as the per
> - // remap instance data.
> + return TSREMAP_NO_REMAP;
> + }
>
> - // Go through all hooks we support, and setup the txn hook(s) as necessary
> - for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> - TSCont contp = NULL;
> + TSRemapStatus rval = TSREMAP_NO_REMAP;
> + RulesConfig* conf = static_cast<RulesConfig*>(ih);
>
> - if (conf->rules[i]) {
> - if (NULL == contp) {
> - contp = TSContCreate(cont_rewrite_headers, NULL);
> - TSContDataSet(contp, conf);
> - }
> - TSHttpTxnHookAdd(rh, static_cast<TSHttpHookID>(i), contp);
> - TSDebug(PLUGIN_NAME, "activated transaction hook via remap.config: new hook=%d", i);
> - }
> + // Go through all hooks we support, and setup the txn hook(s) as necessary
> + for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
> + if (conf->rule(i)) {
> + TSHttpTxnHookAdd(rh, static_cast<TSHttpHookID>(i), conf->continuation());
> + TSDebug(PLUGIN_NAME, "Added remapped TXN hook=%s", HOOK_NAMES[i]);
> }
> + }
>
> - // Now handle the remap specific rules for the "remap hook" (which is not a real hook).
> - // This avoids scheduling an additional continuation for a very common case.
> - RuleSet* rule = conf->rules[TS_REMAP_PSEUDO_HOOK];
> - Resources res(rh, rri);
> -
> - // res.gather(conf->resids[TS_REMAP_PSEUDO_HOOK], TS_REMAP_PSEUDO_HOOK);
> - res.gather(RSRC_CLIENT_REQUEST_HEADERS, TS_REMAP_PSEUDO_HOOK);
> + // Now handle the remap specific rules for the "remap hook" (which is not a real hook).
> + // This is sufficiently differen than the normal cont_rewrite_headers() callback, and
> + // we can't (shouldn't) schedule this as a TXN hook.
> + RuleSet* rule = conf->rule(TS_REMAP_PSEUDO_HOOK);
> + Resources res(rh, rri);
>
> - // Evaluation. This code is duplicated sort of, should we merge with the continuation evaluator ?
> - while (rule) {
> - if (rule->eval(res)) {
> - OperModifiers rt = rule->exec(res);
> + res.gather(RSRC_CLIENT_REQUEST_HEADERS, TS_REMAP_PSEUDO_HOOK);
> + while (rule) {
> + if (rule->eval(res)) {
> + OperModifiers rt = rule->exec(res);
>
> - if (res.changed_url == true)
> - rval = TSREMAP_DID_REMAP;
> + if (res.changed_url == true) {
> + rval = TSREMAP_DID_REMAP;
> + }
>
> - if (rule->last() || (rt & OPER_LAST)) {
> - break; // Conditional break, force a break with [L]
> - }
> + if (rule->last() || (rt & OPER_LAST)) {
> + break; // Conditional break, force a break with [L]
> }
> - rule = rule->next;
> }
> + rule = rule->next;
> }
>
> - TSDebug(PLUGIN_NAME, "returing with status: %d", rval);
> + TSDebug(PLUGIN_NAME_DBG, "Returing from TSRemapDoRemap with status: %d", rval);
> return rval;
> }
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/lulu.h
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/lulu.h b/plugins/header_rewrite/lulu.h
> index 0dd3cb1..eb89031 100644
> --- a/plugins/header_rewrite/lulu.h
> +++ b/plugins/header_rewrite/lulu.h
> @@ -43,8 +43,8 @@
> #error "Define barriers"
> #endif
>
> -extern const char* PLUGIN_NAME;
> -extern const char* PLUGIN_NAME_DBG;
> +extern const char PLUGIN_NAME[];
> +extern const char PLUGIN_NAME_DBG[];
>
>
> // From google styleguide: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/operators.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/operators.cc b/plugins/header_rewrite/operators.cc
> index d6a60e5..3b2cd27 100644
> --- a/plugins/header_rewrite/operators.cc
> +++ b/plugins/header_rewrite/operators.cc
> @@ -361,7 +361,7 @@ OperatorRMHeader::exec(const Resources& res) const
> TSDebug(PLUGIN_NAME, "OperatorRMHeader::exec() invoked on header %s", _header.c_str());
> field_loc = TSMimeHdrFieldFind(res.bufp, res.hdr_loc, _header.c_str(), _header.size());
> while (field_loc) {
> - TSDebug(PLUGIN_NAME, "\tdeleting header %s", _header.c_str());
> + TSDebug(PLUGIN_NAME, " Deleting header %s", _header.c_str());
> tmp = TSMimeHdrFieldNextDup(res.bufp, res.hdr_loc, field_loc);
> TSMimeHdrFieldDestroy(res.bufp, res.hdr_loc, field_loc);
> TSHandleMLocRelease(res.bufp, res.hdr_loc, field_loc);
> @@ -405,7 +405,7 @@ OperatorAddHeader::exec(const Resources& res) const
>
> if (TS_SUCCESS == TSMimeHdrFieldCreateNamed(res.bufp, res.hdr_loc, _header.c_str(), _header.size(), &field_loc)) {
> if (TS_SUCCESS == TSMimeHdrFieldValueStringSet(res.bufp, res.hdr_loc, field_loc, -1, value.c_str(), value.size())) {
> - TSDebug(PLUGIN_NAME, " adding header %s", _header.c_str());
> + TSDebug(PLUGIN_NAME, " Adding header %s", _header.c_str());
> TSMimeHdrFieldAppend(res.bufp, res.hdr_loc, field_loc);
> }
> TSHandleMLocRelease(res.bufp, res.hdr_loc, field_loc);
> @@ -445,7 +445,7 @@ OperatorSetHeader::exec(const Resources& res) const
> // No existing header, so create one
> if (TS_SUCCESS == TSMimeHdrFieldCreateNamed(res.bufp, res.hdr_loc, _header.c_str(), _header.size(), &field_loc)) {
> if (TS_SUCCESS == TSMimeHdrFieldValueStringSet(res.bufp, res.hdr_loc, field_loc, -1, value.c_str(), value.size())) {
> - TSDebug(PLUGIN_NAME, " adding header %s", _header.c_str());
> + TSDebug(PLUGIN_NAME, " Adding header %s", _header.c_str());
> TSMimeHdrFieldAppend(res.bufp, res.hdr_loc, field_loc);
> }
> TSHandleMLocRelease(res.bufp, res.hdr_loc, field_loc);
> @@ -458,7 +458,7 @@ OperatorSetHeader::exec(const Resources& res) const
> if (first) {
> first = false;
> if (TS_SUCCESS == TSMimeHdrFieldValueStringSet(res.bufp, res.hdr_loc, field_loc, -1, value.c_str(), value.size())) {
> - TSDebug(PLUGIN_NAME, " overwriting header %s", _header.c_str());
> + TSDebug(PLUGIN_NAME, " Overwriting header %s", _header.c_str());
> }
> } else {
> TSMimeHdrFieldDestroy(res.bufp, res.hdr_loc, field_loc);
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/parser.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/parser.cc b/plugins/header_rewrite/parser.cc
> index cdff4ee..a389858 100644
> --- a/plugins/header_rewrite/parser.cc
> +++ b/plugins/header_rewrite/parser.cc
> @@ -98,7 +98,7 @@ Parser::preprocess(std::vector<std::string>& tokens)
> Parser::Parser(const std::string& line) :
> _cond(false), _empty(false)
> {
> - TSDebug("header_rewrite_dbg", "Calling CTOR for Parser");
> + TSDebug(PLUGIN_NAME_DBG, "Calling CTOR for Parser");
>
> if (line[0] == '#') {
> _empty = true;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/resources.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/resources.cc b/plugins/header_rewrite/resources.cc
> index d89149a..eb4afaf 100644
> --- a/plugins/header_rewrite/resources.cc
> +++ b/plugins/header_rewrite/resources.cc
> @@ -24,10 +24,15 @@
> #include "resources.h"
> #include "lulu.h"
>
> +// This is defined in header_rewrite.cc, used here for debugging
> +extern char* HOOK_NAMES[];
> +
> +
> +// Collect all resources
> void
> Resources::gather(const ResourceIDs ids, TSHttpHookID hook)
> {
> - TSDebug(PLUGIN_NAME, "Building resource structure for hook (%d)", hook);
> + TSDebug(PLUGIN_NAME, "Building resources, hook=%s", HOOK_NAMES[hook]);
>
> // If we need the client request headers, make sure it's also available in the client vars.
> if (ids & RSRC_CLIENT_REQUEST_HEADERS) {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/resources.h
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/resources.h b/plugins/header_rewrite/resources.h
> index cb9d062..5bda0da 100644
> --- a/plugins/header_rewrite/resources.h
> +++ b/plugins/header_rewrite/resources.h
> @@ -60,7 +60,6 @@ public:
> _rri(rri), changed_url(false), _ready(false)
> {
> TSDebug(PLUGIN_NAME_DBG, "Calling CTOR for Resources (RemapAPI)");
> - TSDebug(PLUGIN_NAME, "rri: %p", _rri);
> }
>
> ~Resources() { destroy(); }
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/31792d2a/plugins/header_rewrite/ruleset.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/header_rewrite/ruleset.cc b/plugins/header_rewrite/ruleset.cc
> index 9c6bc06..e210e47 100644
> --- a/plugins/header_rewrite/ruleset.cc
> +++ b/plugins/header_rewrite/ruleset.cc
> @@ -47,7 +47,7 @@ RuleSet::add_condition(Parser& p)
> Condition* c = condition_factory(p.get_op());
>
> if (NULL != c) {
> - TSDebug(PLUGIN_NAME, "Adding condition: %%{%s} with arg: %s\n", p.get_op().c_str(), p.get_arg().c_str());
> + TSDebug(PLUGIN_NAME, " Adding condition: %%{%s} with arg: %s\n", p.get_op().c_str(), p.get_arg().c_str());
> c->initialize(p);
> if (!c->set_hook(_hook)) {
> TSError("%s: can't use this condition in this hook", PLUGIN_NAME);
> @@ -73,7 +73,7 @@ RuleSet::add_operator(Parser& p)
>
> if (NULL != o) {
> // TODO: This should be extended to show both the "argument" and the "value" (if both are used)
> - TSDebug(PLUGIN_NAME, "Adding operator: %s(%s)\n", p.get_op().c_str(), p.get_arg().c_str());
> + TSDebug(PLUGIN_NAME, " Adding operator: %s(%s)\n", p.get_op().c_str(), p.get_arg().c_str());
> o->initialize(p);
> if (!o->set_hook(_hook)) {
> TSError("%s: can't use this operator in this hook", PLUGIN_NAME);
>
Re: [1/2] git commit: TS-2522 Better hook management, and cleanup
Posted by Leif Hedstrom <zw...@apache.org>.
On Mar 4, 2014, at 4:36 PM, James Peach <jp...@apache.org> wrote:
>> +const char PLUGIN_NAME[] = "header_rewrite";
>> +const char PLUGIN_NAME_DBG[] = "dbg_header_rewrite";
>> +
>> +const char* HOOK_NAMES[] = {
>
> You should do some kind of static assert to force a build break if new hooks are added.
I’m hoping to replace this with the new APIs we’ve been discussing. This junk is the reason why I filed that Jira :).
— Leif
Re: [1/2] git commit: TS-2522 Better hook management, and cleanup
Posted by Leif Hedstrom <zw...@apache.org>.
On Mar 4, 2014, at 4:36 PM, James Peach <jp...@apache.org> wrote:
>> +const char PLUGIN_NAME[] = "header_rewrite";
>> +const char PLUGIN_NAME_DBG[] = "dbg_header_rewrite";
>> +
>> +const char* HOOK_NAMES[] = {
>
> You should do some kind of static assert to force a build break if new hooks are added.
I’m hoping to replace this with the new APIs we’ve been discussing. This junk is the reason why I filed that Jira :).
— Leif