On Nov 12, 2013, at 3:27 PM, bri...@apache.org wrote: > TS-2316: header_rewrite: fixed segfaults in ConditionPath::append_value() > > -void > -ConditionPath::append_value(std::string& s, const Resources& res) > -{ > - int path_len = 0; > - const char *path = TSUrlPathGet(res._rri->requestBufp, > res._rri->requestUrl, &path_len); > - TSDebug(PLUGIN_NAME, "Appending PATH to evaluation value: %.*s", path_len, > path); > - s.append(path, path_len); > +void ConditionPath::append_value(std::string& s, const Resources& res) {
Sad panda; why did you undo the consistent changes from a previous patch? The { should be on its own line, no ? > + TSMBuffer bufp; > + TSMLoc url_loc; > + > + if (TSHttpTxnPristineUrlGet(res.txnp, &bufp, &url_loc) == TS_SUCCESS) { > + int path_length; > + const char *path = TSUrlPathGet(bufp, url_loc, &path_length); Also, I’m not sure why this change fixes a segfault? I.e. why the change from using the rri->requestUrl to use TSHttpTxnPristineUrlGet ? Is that to make it work with both remap plugins and global hooks? If so, the commit comment is feather misleading. What I’d probably prefer to see is to use the “rri” when possible, and other APIs when not. The “rri” structure is only populated in remap plugins (of course), but we should use it for efficiency when available (IMO at least). — Leif