Attempting to fix issue #29 #31
Conversation
|
I don't think that this is correct. When the text becomes fragmented, the start of the command gets lost (i.e. You get around 96 "Invalid command" responses, and 3 for "Text has been indexed". I think it's because currently, the leftover gets thrown away. It should be kept instead of |
|
Yeah, you’re right. I rushed this fix a bit. I will update this PR hopefully today at some point with the correct solution. |
|
Okay, just pushed the fix for #29 and did some more thorough testing. It seems to be working now properly. Printing the commands as they enter |
| if(should_close) { | ||
| close(i); | ||
| FD_CLR(i, &master_fds); | ||
| dfree(connection_infos[i].last_command); |
There was a problem hiding this comment.
I think this should break from the loop, otherwise we'll try to write to a socket that has been closed, and try to access a string that has been freed.
There was a problem hiding this comment.
True, I added the break in this if statement. Should be good to go now.
@00-matt This PR is attempting to fix the issue discussed #23 (comment)
What I am doing is reading the
this->last_commandvariable before it is sent toprocess_commandand splitting it at\r\nand sending everything before the new line characters.What I am worried about is that after the loop is done I am sending what is left of the
commandvariable which could be only a fragment of the original block of text for examplepig in a cagewhen fragmented could becomeig in a cageleading to false indices. I haven't been able to produce this issue when testing but I wanted to get a second set of eyes on the code before I merged it.