Skip to content
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

updated types to bool in _turn_params_ to reflect C11 #1406

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

redraincatching
Copy link
Contributor

@redraincatching redraincatching commented Jan 26, 2024

approach was as follows, for the _turn_params_ struct:

  • if a variable of type int or vint was only being used as a boolean, replace it with bool as defined in <stdbool.h>
  • replace its declaration with true/false, depending on prior assignment as 0/1

changes were only made when i was certain the variables were not being used as an int, so i may have missed some

no changes were made to other sections of the code as int-to-bool assignment is allowed in C, only code within the structs were changed, but that can be changed with a later commit


from a documentation perspective, it's not clear as to what purpose or benefit the vint alias has. the definition in ns_turn_defs.h simply reads

typedef int vint;
typedef vint *vintp;

with no comments, and it seems most (but not all) vints are being used as interim booleans through the code. this may just be from lack of knowledge of the codebase, but it doesn't seem useful in any way, so it would be helpful if someone with more expertise could clarify

@eakraly
Copy link
Collaborator

eakraly commented Jan 28, 2024

Thank you @redraincatching for your contribution! coturn code will greatly benefit from modernization

@eakraly
Copy link
Collaborator

eakraly commented Jan 28, 2024

And the lint issue is because of formatting change that just merged - sorry about the rework you need to do!

@redraincatching
Copy link
Contributor Author

@eakraly that formatting issue should be sorted now - glad to hear the changes are being well received. i hope to do the same to a few more files over the next while, too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants