On Mon, 23 Aug 2010 16:47:28 +0200, René Hummen
<[email protected]> wrote:
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).
_______________________________________________
Mailing list: https://launchpad.net/~hipl-core
Post to : [email protected]
Unsubscribe : https://launchpad.net/~hipl-core
More help : https://help.launchpad.net/ListHelp
