Use enums for tri-state return values #135
Conversation
src/SDL_net.c
Outdated
| @@ -1350,15 +1356,15 @@ static int SendOneDatagram(NET_DatagramSocket *sock, NET_Address *addr, Uint16 p | |||
| } | |||
|
|
|||
| SDL_assert(rc == buflen); | |||
| return 1; | |||
| return NET_SUCCESS; | |||
| } | |||
|
|
|||
| // see if any pending data can finally be sent, etc | |||
| static int PumpDatagramSocket(NET_DatagramSocket *sock) | |||
There was a problem hiding this comment.
Should the return type be enum TriState?
There was a problem hiding this comment.
Let's change PumpDatagramSocket to return bool (this was written before SDL3 changed from 0-or-negative-one to true-or-false for these sort of things).
I can do that separately (in which case, just back out changes to this function), or you can change it in this PR. Either's fine with me.
There was a problem hiding this comment.
I wouldn't mind doing that, but I'll do it in a separate PR. Since it's going to return a bool, would you like the function name changed a little as well?
typedef enum NET_TriState
{
Blah;
} NET_TriState;(needs to be this way for the wiki bridge and consistency). Let's call it something like NET_Outcome or NET_Result instead of NET_TriState. I'm open to suggestions ("result" sort of implies everything should return this type, which isn't good). Outside of the typedef, never use the |
Done. I changed it to NET_Status. I don't have any other suggestions at this time. |
| { | ||
| struct addrinfo *addrwithport = MakeAddrInfoWithPort(addr, SOCK_DGRAM, port); | ||
| if (!addrwithport) { | ||
| return -1; | ||
| return NET_FAILURE; |
There was a problem hiding this comment.
I missed this last time, but below this, you are returning 0 instead of NET_WOULDBLOCK.
There was a problem hiding this comment.
@mkunkacnc You're referring to return WouldBlock(err) ? 0? That's used in two other locations, in functions that wouldn't apply to this PR. To maintain some consistency, I'm not sure that should be changed there (although it wouldn't hurt and it would have the same effect).
Actually, I will make that change in this function.
It might be that there needs to be a discussion on whether all the zeroes in the code that indicate NET_WOULDBLOCK should be changed at some point.
@icculus Where do you stand on this?
|
@icculus I've searched the code some, but it's not clear to me yet if any other functions need to be changed. It might be more efficient for someone else to finish this than for you to try to point out any remaining changes needed. Any idea on how much stuff I missed? |
|
Let's merge this. If there are other things to tweak, we can always make more commits. :) |
Resolves #112
There's probably some other areas that could be changed as well, but I thought I'd submit what I have so far for review and feedback.