[
https://issues.apache.org/jira/browse/TS-4054?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15050057#comment-15050057
]
ASF GitHub Bot commented on TS-4054:
------------------------------------
Github user jpeach commented on the pull request:
https://github.com/apache/trafficserver/pull/363#issuecomment-163487199
AFAICT the assertion in ```setup_diagslog``` is correct. I would even go so
far as to make it an ```ink_release_assert``` because otherwise we would leak
the global ```diags_log```. I can see a couple of places where ```diags_log```
is deleted before ```setup_diagslog``` is called.
The best way to fix this IMHO is to not deal with ```drags_log``` at all in
```setup_diagslog```. The pattern that this code should do is:
if (setup_diagslog(blf)) {
delete drags_log;
diags_log = blf;
}
```setup_diagslog``` can be changed from a member function to a local
static function, and you can remove all mentions of ```diags_log``` from it.
> Incorrect ink_assert behavior in Diags.cc
> -----------------------------------------
>
> Key: TS-4054
> URL: https://issues.apache.org/jira/browse/TS-4054
> Project: Traffic Server
> Issue Type: Bug
> Components: Logging
> Reporter: Daniel Xu
> Fix For: 6.1.0
>
>
> In `lib/ts/Diags.cc:setup_diagslog()`, the statement `ink_assert(diags_log ==
> NULL)` is incorrect. It should instead be: `if (blf == NULL) return`
> We should not use an assert here either because when the function is passed a
> NULL `blf`, it means that some test doesn't want to use the diags.log log
> file and we should just not set up diagslog.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
