enable persistent view of irc messages #362

Merged
merged 3 commits into from Feb 2, 2017

Projects

None yet

2 participants

@deep-42-thought
Contributor
  • remove directory from libirc-include
  • give possibility to specify number-of-last-lines to show for irc instead of showing all lines since last time ${irc server(:port) #channel (number-of-lines)}
  • is backwards compatible: omitting number-of-lines resorts to old behaviour
src/irc.cc
+ if(ctxptr->max_msg_lines>0) {
+ int msgcnt = 0;
+ newmsg = ctxptr->messages;
+ while(newmsg) {
@brndnmtthws
brndnmtthws Jan 31, 2017 Owner

There needs to be some sort of bounds checking here.

@deep-42-thought
deep-42-thought Jan 31, 2017 Contributor

I'm not sure, what variable should be checked. The loop is pretty similar to the one in line 68..70. Or do you mean, I should check for an overflow of msgcnt? This, however, only increases by one each time, the function is called, and gets truncated to ctxptr->max_msg_lines at the end of the function call iff ctxptr->max_msg_lines>0. Should I prepare the function for a changing ctxptr->max_msg_lines, so it may switch from 0 to some positive value while the current message count is larger than max(int)?

@brndnmtthws
brndnmtthws Jan 31, 2017 Owner

I'm wondering what happens if there's a case where there's no null pointer in newmsg. Rather than having to read through all the code, I prefer to see some kind of bounds check. This is how security vulnerabilities are created (although I'm not saying you created one, I'm just being conservative).

@deep-42-thought
deep-42-thought Jan 31, 2017 Contributor

ok, I can assume some upper bound for max_msg_lines and msg_count - this should be reasonable. However, the loop in line 68 to 70 will last indefinitely if there is no null pointer, anyway.

@deep-42-thought
deep-42-thought Jan 31, 2017 Contributor

Is the approach in bcaeffd something you had in mind?

src/irc.cc
+ msgcnt++;
+ }
+ msgcnt -= ctxptr->max_msg_lines;
+ while(msgcnt>0) {
@brndnmtthws
brndnmtthws Jan 31, 2017 Owner

Maybe here too?

@brndnmtthws brndnmtthws merged commit e84ca1f into brndnmtthws:master Feb 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment