[jira] [Commented] (TS-4054) Incorrect ink_assert behavior in Diags.cc

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/TS-4054?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15041822#comment-15041822 ] 

ASF GitHub Bot commented on TS-4054:
------------------------------------

GitHub user danobi opened a pull request:

    https://github.com/apache/trafficserver/pull/363

    TS-4054 Incorrect ink_assert behavior in Diags.cc

    `ink_assert(diags_log == NULL)` is incorrect since there are times
    when `blf` correctly has the value NULL. In that case, we would
    like for the function to do nothing and return early versus crash
    ATS via an assert. That is to say, semantically an early return is
    more clear than an assert.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/danobi/trafficserver TS-4054

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafficserver/pull/363.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #363
    
----
commit 0191a4f367d868409b9b4f2935ecfdaa02414615
Author: Daniel Xu <dl...@yahoo.com>
Date:   2015-12-04T17:28:47Z

    TS-4054 Incorrect ink_assert behavior in Diags.cc
    
    `ink_assert(diags_log == NULL)` is incorrect since there are times
    when `blf` correctly has the value NULL. In that case, we would
    like for the function to do nothing and return early versus crash
    ATS via an assert. That is to say, semantically an early return is
    more clear than an assert.

----


> 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)