| Thread Previous • Date Previous • Date Next • Thread Next |
Did you check which error was detected initially? While fixing the side-effect on R2 handling, it might also be necessary to have a look at the original error.
Fortunately, it worked right out of the box on Samuel's setup so there's logs from succesful runs. I didn't spot any less (reported) errors than in my logs so far though, but I'm going to investigate more closely.
Is ctx->error reset at all? My grep-fu didn't reveal anything. All other fields seem to be reset properly. Not at a single place but as-needed though.
This is a good start, but your patch should integrate cleanly with the structure of the handle_functions, in order for other developers to be able to read and understand the code easily. Your patch "hides" the re-initialization somewhere in the code.
OK you're right, it can't stay that way.
As such, ctx->error has a more global scope than mere socket errors and should therefore not be re-initialized in hipd/hip_socket.c. You should move the re-initialization to the beginning of the while loop of hipd/hipd.c:383 (...) and re-initialize all members of ctx that are required to be initialized to 0/NULL.
But errors encountered while processing one socket are going to spill into another socket if we cleanup solely in hipd_main, because ctx->error is going to be retained between invocations at hipd/hip_socket.c:280. On the other hand, callbacks handle at most one packet and rely on being called again with the remaining buffer, so there would be no point in clearing at a lower level either.
Imho, ctx is semantically out of place in hipd_main (not used, apart from one misuse of ctx->input_msg as a temporary buffer ;) but reallocating it every time at a lower level would hit performance, even if statically scoped, so very first initialization should stay in hipd_main.
This should solve similar bugs with different members of ctx as well.
We could place some asserts there to ensure buffers were freed too, instead of "cleaning up" what isn't supposed to be there in the first place (hipd/hipd.c:436).
| Thread Previous • Date Previous • Date Next • Thread Next |
