bneradt commented on code in PR #10817:
URL: https://github.com/apache/trafficserver/pull/10817#discussion_r1397814856
##########
plugins/experimental/maxmind_acl/mmdb.cc:
##########
@@ -78,11 +78,16 @@ Acl::init(char const *filename)
return status;
}
- // Associate our config file with remap.config to be able to initiate reloads
+ // Associate our config file with remap.config if possible to be able to
initiate reloads
TSMgmtString result;
const char *var_name = "proxy.config.url_remap.filename";
- TSMgmtStringGet(var_name, &result);
- TSMgmtConfigFileAdd(result, configloc.c_str());
+ if (TS_SUCCESS == TSMgmtStringGet(var_name, &result)) {
+ if (TS_SUCCESS != TSMgmtConfigFileAdd(result, configloc.c_str())) {
+ TSWarning("[%s] Error adding mgmt config file", PLUGIN_NAME);
+ }
+ } else {
+ TSWarning("[%s] Could not retrieve remap filename", PLUGIN_NAME);
+ }
Review Comment:
This is super nit picky, but we can keep the boolean checks consistent and
avoid some nesting like so:
```cpp
if (TS_SUCCESS != TSMgmtStringGet(var_name, &result)) {
TSWarning("[%s] Could not retrieve remap filename", PLUGIN_NAME);
} else if (TS_SUCCESS != TSMgmtConfigFileAdd(result, configloc.c_str())) {
TSWarning("[%s] Error adding mgmt config file", PLUGIN_NAME);
}
```
I'll leave it up to you. I'll approve as is if you want to go forward with
what you have.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
