Skip to content

Conversation

@ngara
Copy link
Contributor

@ngara ngara commented Oct 26, 2016

Please review - I changed this code to ink_release_assert instead of the following code, which seems to either return '\0' or *b->start(); but will ink_assert in debug mode.

ink_assert(!"out of range");
const: At condition !!b, the value of b must be equal to 0.
null: At condition !!b, the value of b must be NULL.
dead_error_condition: The condition !!b cannot be true.
700 if (unlikely(b)) {

CID 1022011 (#1 of 1): Logically dead code (DEADCODE)
dead_error_line: Execution cannot reach this statement: return b->start();.
701 return *b->start();
702 }

@zwoop zwoop added the Core label Oct 26, 2016
@zwoop zwoop added this to the 7.1.0 milestone Oct 26, 2016
@zwoop
Copy link
Contributor

zwoop commented Oct 27, 2016

[approve ci]

@atsci
Copy link

atsci commented Oct 27, 2016

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/1004/ for details.

@atsci
Copy link

atsci commented Oct 27, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1111/ for details.

@zwoop
Copy link
Contributor

zwoop commented Oct 27, 2016

@ngara I think this is failing because of clang-format. I recommend you copy the pre-commit githook into your .git/hooks tree.

@ngara
Copy link
Contributor Author

ngara commented Oct 27, 2016

I put the plugin hook in and got it working. Thanks for the suggestion.

Nathan

On Oct 26, 2016, at 9:01 PM, Leif Hedstrom notifications@github.com wrote:

@ngara https://github.com/ngara I think this is failing because of clang-format. I recommend you copy the pre-commit githook into your .git/hooks tree.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #1152 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA51eE0tGOe6QyG0UPgODLOK41L_-Xutks5q4CILgaJpZM4KhylU.

@zwoop
Copy link
Contributor

zwoop commented Oct 27, 2016

[approve ci]

@ngara
Copy link
Contributor Author

ngara commented Oct 27, 2016

I took a close look at this and I didn't make the change that caused clang-format to fail here

@atsci
Copy link

atsci commented Oct 27, 2016

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/1008/ for details.

@ngara
Copy link
Contributor Author

ngara commented Oct 27, 2016

OK, figured it out. removing a variable made the equals signs want to line up differently on a line i didnt modify. Fixed!

@atsci
Copy link

atsci commented Oct 27, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/1114/ for details.

@ngara
Copy link
Contributor Author

ngara commented Oct 27, 2016

Maybe this should be an ink_abort instead of ink_release_assert?

Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me, I will commit unless someone else disagrees.

@zwoop zwoop self-assigned this Nov 1, 2016
@bryancall bryancall merged commit 2ae0497 into apache:master Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants