-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FreeRDP master && plain C++ #45
base: master
Are you sure you want to change the base?
Conversation
Cast function pointer arguments in implementation so that changes in functin pointer arguments emit warnings
* do not cast function pointers, use proper arguments and cast them inside the function to appropriate types. * do not declare function prototypes mid code, move them to a header
do not start include guards with underscore, that is a reserved keyword
switch (service->messageType) { | ||
case OGON_CLIENT_CAPABILITIES: | ||
IFCALLRET( | ||
client->Capabilities, success, cb_data, &msg->capabilities); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hum pretty ugly code style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately the old code did violate lots of format string requirements (e.g. "sometext %" PRIx32
requires the space between the format string and the PRIx32
macro), so the reformatting was required.
UINT16 using_buckets = (UINT16)conn->front.frameAcknowledge; | ||
|
||
if (!using_buckets) { | ||
using_buckets = (UINT16)((conn->fps / 2 > OGON_MAX_BUCKET) ? OGON_MAX_BUCKET : conn->fps / 2); | ||
using_buckets = | ||
(UINT16)((conn->fps / 2 > OGON_MAX_BUCKET) ? OGON_MAX_BUCKET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugly code style
frameack = MIN( | ||
MAX( peer->autodetect->netCharBaseRTT * connection->fps / 1000, 2 ), | ||
(unsigned int) connection->fps ); | ||
frameack = MIN(MAX(peer->context->autodetect->netCharBaseRTT * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previous code style was easier to read (perhaps that shall be splitted in 2 operations)
WLog_DBG(TAG, "------------------------------------------------------------+-------"); | ||
WLog_DBG(TAG, "STOPWATCH | COUNT | TOTAL | AVG | IPS"); | ||
WLog_DBG(TAG, "---------------------+------------+-------------+-----------+-------"); | ||
WLog_DBG(TAG, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code style change is quite unfortunate here
@@ -386,59 +391,63 @@ static BOOL ogon_peer_post_connect(freerdp_peer *client) | |||
return FALSE; | |||
} | |||
|
|||
if (client->settings->AutoLogonEnabled) { | |||
if (client->context->settings->AutoLogonEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here and below that could be simply settings->
instead of client->context->settings->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some remarks, but LGTM.
If we act to go to C++, then probably that using the C++ binding of protobuf would do some nicer code for the backend communication (the C binding produces so long names)
@hardening did not have time yet to get that working. |
Yeah no pb, that PR is already super huge. I think that would be enough for migration beginning. |
No description provided.