Tcp #67
Tcp #67
Conversation
|
Yes, you can start with async. I can take a look into this. Thanks. |
|
@jcarreira for the mem_exhaustion test, was the issue that the exception was not being thrown, or that an unexpected exception was being thrown? The client side exception should be generated in |
|
Need to fix #72 before merging this PR. We need |
|
Alright, I'll trying get #72 resolved by the end of today. Should I do it within this PR, or should I make a separate pull request to merge it into this branch? |
src/common/Future.cpp
Outdated
| */ | ||
| bool Future::try_wait() { | ||
| if (!result_available) { | ||
| bool sem_success = sem->trywait(); |
There was a problem hiding this comment.
Why not return result_available = sem->trywait()?
There was a problem hiding this comment.
Reading the man page for trywait on a posix semaphore, it sounds as if it only returns true if the value of the semaphore is currently positive. As such, I thought that this was better behavior in case of repeated calls to the try_wait() method, even after it has succeeded, because it will return true if trywait has ever returned true.
There was a problem hiding this comment.
The comment was related to the code inside the first if block.
There was a problem hiding this comment.
Ah, I'll go ahead and do that
|
You can put it into this PR. |
|
@jcarreira the latest commits resolve #72 . |
src/common/Future.cpp
Outdated
| } | ||
| case cirrus::ErrorCodes::kException: { | ||
| throw cirrus::Exception("Server threw generic exception."); | ||
| break; |
There was a problem hiding this comment.
This break seems unnecessary because exception is thrown in the previous line.
There was a problem hiding this comment.
I'm assuming this applies to all of the exceptions in this switch case, so I'll remove all of those break statements
src/server/bladeallocmain.cpp
Outdated
| std::cout << "Pool size in invalid format." << std::endl; | ||
| } | ||
| } else { | ||
| std::cout << "Pass desired poolsize in bytes" << std::endl; |
There was a problem hiding this comment.
Should print something like: "Error: ./bladeallocmain [pool_size]"
|
The PR is close to ready. Let me know once those 2 last comments are addressed and I then I will merge this. |
|
@jcarreira The latest commit should have addressed your comments. I also changed the server main files to return an error status if arguments are provided incorrectly |
Working towards accomplishing #9 .
Changes
Issues
To Later Address