Constness TCPSocket::read/write methods #157
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #157 +/- ##
========================================
Coverage 63.74% 63.75%
========================================
Files 1066 1066
Lines 55360 55353 -7
Branches 4086 4086
========================================
Hits 35289 35289
+ Misses 20071 20064 -7 ☔ View full report in Codecov by Sentry. |
|
Private downstream CI failed. |
src/eckit/net/TCPSocket.h
Outdated
| char mode_; | ||
| mutable bool debug_; | ||
| mutable bool newline_; | ||
| mutable char mode_; |
There was a problem hiding this comment.
This doesn't look right.
mode_ (IIRC) should only be being set in the open() methods (which absolutely should not be const).
There was a problem hiding this comment.
it's an unfortunate result of the following (in read/write methods);
if (debug_) {
...
if (mode_ != 'r') {
newline_ = true;
mode_ = 'r';
}
...
}There was a problem hiding this comment.
all those members are used in if(debug) blocks, so what's the concern here ?
There was a problem hiding this comment.
it should be safer and easier to see after my recent commit
There was a problem hiding this comment.
Ah, I hadn't realised that mode_ was only used for debugging. That is a peculiar choice. This updated PR makes that much clearer. Thanks.
for better visibility this stuff should not have imposed non-const-ness on methods
|
Private downstream CI succeeded. |
|
Private downstream CI failed. |
|
Private downstream CI succeeded. |
1 similar comment
|
Private downstream CI succeeded. |
|
Private downstream CI succeeded. |
1 similar comment
|
Private downstream CI succeeded. |
|
Private downstream CI succeeded. |
1 similar comment
|
Private downstream CI succeeded. |
No description provided.