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

Implement proper exception handling #38

Open
amlweems opened this issue Oct 12, 2014 · 6 comments
Open

Implement proper exception handling #38

amlweems opened this issue Oct 12, 2014 · 6 comments
Assignees

Comments

@amlweems
Copy link
Contributor

It is possible for our client to be incredibly confused to the point of being unable to correctly parse any server messages (you have no idea how easy it is for this to happen). This can cause segfaults, buffer overflows, and generally is very bad. In these situations, it is very likely that we can detect an error.

We should implement proper exception handling in these cases (possibly even disconnecting and reconnecting the client).

@JDongian
Copy link
Member

Also put documentation on doxygen and/or the README.md

@JDongian
Copy link
Member

Can you give a sample implementation of exception handling? What would this look like, and what cases would exceptions cover?

@amlweems
Copy link
Contributor Author

Not a code sample, but these are the important locations where exceptions will come in handy.

https://github.com/NosotrosNueces/mcc/blob/master/src/protocol.c#L574

// Negative packet IDs probably mean something terrible happened... so we just exit
int32_t pid = receive_packet(bot);
if (pid < 0) {
    if (pid == -1) exit(123);
    return NULL;
}

https://github.com/NosotrosNueces/mcc/blob/master/src/marshal.c#L263

varlen = varint32_encode(packet_raw - save, varint, 5);
if(packet_raw - save + varlen > len)
    return -1; // TODO: compression

https://github.com/NosotrosNueces/mcc/blob/master/src/bot.c#L152

// A packet length of zero can sometimes happen, we return -2
len = varint32(bot->_data->buf, &packet_size);
if (packet_size == 0)
    return -2;
assert(i != len);
...
// This pattern occurs 3 times in receive_packet()
ret = receive_raw(bot, bot->_data->buf + received, packet_size - received);
if (ret <= 0)
    return -1;
...
// If we can't parse a big packet correctly, we return -2
return -2;

@JDongian JDongian assigned JDongian and unassigned SrsBusiness Dec 27, 2014
@JDongian
Copy link
Member

Thanks @amlweems. I'll be working on this. The right approach may be to use jumps, i.e. setjump() and longjump(), but I am skeptical.

Error handling with signals seems a lot better IMO.

idea:

set errno (global)
send interrupt
signal handler logs message based on errno
signal handler kills main?

@JDongian
Copy link
Member

JDongian commented Jan 8, 2015

Got some basic stuff working on branch exceptions #80. Should I put the exception handler in a separate file? Where should it go? sample/ or src/? Perhaps we could macro the call?

@JDongian
Copy link
Member

Okay, what's the status on this? I want to close this and move on.

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

No branches or pull requests

3 participants