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


Reply via email to