)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1000002,"name":"cron2","display_name":"Gert Doering","email":"gert@greenie.muc.de","username":"cron2"},"change_message_id":"26ae51594b7447d382477fac1ca2de0f0eec31c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a93a9c41_abd969c9","updated":"2024-12-20 21:56:39.000000000","message":"This looks like it should work (I do have a few nags)","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"}],"src/openvpn/forward.c":[{"author":{"_account_id":1000002,"name":"cron2","display_name":"Gert Doering","email":"gert@greenie.muc.de","username":"cron2"},"change_message_id":"26ae51594b7447d382477fac1ca2de0f0eec31c8","unresolved":true,"context_lines":[{"line_number":1235,"context_line":"    struct link_socket_actual *remote \u003d c-\u003ec2.to_link_addr;"},{"line_number":1236,"context_line":"    const sa_family_t family \u003d incoming-\u003edest.addr.sa.sa_family;"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    if (remote \u0026\u0026 (family \u003d\u003d AF_INET || family \u003d\u003d AF_INET6))"},{"line_number":1239,"context_line":"    {"},{"line_number":1240,"context_line":"        floated \u003d !link_socket_actual_match(incoming, remote);"},{"line_number":1241,"context_line":"    }"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"54eabdc3_8b8edf10","line":1238,"updated":"2024-12-20 21:56:39.000000000","message":"I\u0027m not sure what *that* does?  When can we ever have a `remote` and an family that is neither `AF_INET` nor `AF_INET6`?  (If we\u0027re waiting for incoming data it can be `AF_UNSPEC`, but then we have no `remote`, no?)","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000041,"name":"ralf_lici","display_name":"Ralf Lici","email":"ralf@mandelbit.com","username":"ralf_lici"},"change_message_id":"85d1cbd99244998c8632fb89a095d70d03466b48","unresolved":false,"context_lines":[{"line_number":1235,"context_line":"    struct link_socket_actual *remote \u003d c-\u003ec2.to_link_addr;"},{"line_number":1236,"context_line":"    const sa_family_t family \u003d incoming-\u003edest.addr.sa.sa_family;"},{"line_number":1237,"context_line":""},{"line_number":1238,"context_line":"    if (remote \u0026\u0026 (family \u003d\u003d AF_INET || family \u003d\u003d AF_INET6))"},{"line_number":1239,"context_line":"    {"},{"line_number":1240,"context_line":"        floated \u003d !link_socket_actual_match(incoming, remote);"},{"line_number":1241,"context_line":"    }"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"a2f4224f_180af603","line":1238,"in_reply_to":"54eabdc3_8b8edf10","updated":"2024-12-21 15:23:08.000000000","message":"Acknowledged","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000002,"name":"cron2","display_name":"Gert Doering","email":"gert@greenie.muc.de","username":"cron2"},"change_message_id":"26ae51594b7447d382477fac1ca2de0f0eec31c8","unresolved":true,"context_lines":[{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"    if (process_incoming_link_part1(c, lsi, floated))"},{"line_number":1244,"context_line":"    {"},{"line_number":1245,"context_line":"        if (floated \u0026\u0026 c-\u003ec2.buf.len \u003e 0)"},{"line_number":1246,"context_line":"        {"},{"line_number":1247,"context_line":"            msg(D_LOW, \"peer floated from %s to %s\","},{"line_number":1248,"context_line":"                print_link_socket_actual(remote, \u0026gc),"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"c48925bd_4bc7c300","line":1245,"updated":"2024-12-20 21:56:39.000000000","message":"the GC seems only used inside here, and since this is a performance relevant function (\"hot path\", executed for each packet) we should not doing unneccessary work -\u003e `gc_new(), gc_free()` can be inside the `if (floated)`.","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000041,"name":"ralf_lici","display_name":"Ralf Lici","email":"ralf@mandelbit.com","username":"ralf_lici"},"change_message_id":"85d1cbd99244998c8632fb89a095d70d03466b48","unresolved":false,"context_lines":[{"line_number":1242,"context_line":""},{"line_number":1243,"context_line":"    if (process_incoming_link_part1(c, lsi, floated))"},{"line_number":1244,"context_line":"    {"},{"line_number":1245,"context_line":"        if (floated \u0026\u0026 c-\u003ec2.buf.len \u003e 0)"},{"line_number":1246,"context_line":"        {"},{"line_number":1247,"context_line":"            msg(D_LOW, \"peer floated from %s to %s\","},{"line_number":1248,"context_line":"                print_link_socket_actual(remote, \u0026gc),"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ed3d6af5_b5a33468","line":1245,"in_reply_to":"c48925bd_4bc7c300","updated":"2024-12-21 15:23:08.000000000","message":"Definitely, I think I forgot to reorder this when I removed the other debugging prints I used for testing purposes. Acknowledged.","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000041,"name":"ralf_lici","display_name":"Ralf Lici","email":"ralf@mandelbit.com","username":"ralf_lici"},"change_message_id":"85d1cbd99244998c8632fb89a095d70d03466b48","unresolved":false,"context_lines":[{"line_number":1248,"context_line":"                print_link_socket_actual(remote, \u0026gc),"},{"line_number":1249,"context_line":"                print_link_socket_actual(incoming, \u0026gc));"},{"line_number":1250,"context_line":"            link_socket_set_outgoing_addr(lsi, \u0026c-\u003ec2.from, NULL, c-\u003ec2.es);"},{"line_number":1251,"context_line":"            tls_update_remote_addr(c-\u003ec2.tls_multi, incoming);"},{"line_number":1252,"context_line":"        }"},{"line_number":1253,"context_line":"        process_incoming_link_part2(c, lsi, orig_buf);"},{"line_number":1254,"context_line":"    }"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"34c401d6_00f96d38","line":1251,"range":{"start_line":1251,"start_character":12,"end_line":1251,"end_character":34},"updated":"2024-12-21 15:23:08.000000000","message":"Tested the non-tls p2p configuration (`--secret`) and realized that this line obviously segfaults. Sorry for this, I will fix it on the next patch.","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000002,"name":"cron2","display_name":"Gert Doering","email":"gert@greenie.muc.de","username":"cron2"},"change_message_id":"26ae51594b7447d382477fac1ca2de0f0eec31c8","unresolved":true,"context_lines":[{"line_number":1250,"context_line":"            link_socket_set_outgoing_addr(lsi, \u0026c-\u003ec2.from, NULL, c-\u003ec2.es);"},{"line_number":1251,"context_line":"            tls_update_remote_addr(c-\u003ec2.tls_multi, incoming);"},{"line_number":1252,"context_line":"        }"},{"line_number":1253,"context_line":"        process_incoming_link_part2(c, lsi, orig_buf);"},{"line_number":1254,"context_line":"    }"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":"    gc_free(\u0026gc);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"21341812_f18c537c","line":1253,"updated":"2024-12-20 21:56:39.000000000","message":"should this be *inside* the `if()`?  In the old code `_part2()` is unconditionally called always","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000041,"name":"ralf_lici","display_name":"Ralf Lici","email":"ralf@mandelbit.com","username":"ralf_lici"},"change_message_id":"85d1cbd99244998c8632fb89a095d70d03466b48","unresolved":true,"context_lines":[{"line_number":1250,"context_line":"            link_socket_set_outgoing_addr(lsi, \u0026c-\u003ec2.from, NULL, c-\u003ec2.es);"},{"line_number":1251,"context_line":"            tls_update_remote_addr(c-\u003ec2.tls_multi, incoming);"},{"line_number":1252,"context_line":"        }"},{"line_number":1253,"context_line":"        process_incoming_link_part2(c, lsi, orig_buf);"},{"line_number":1254,"context_line":"    }"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":"    gc_free(\u0026gc);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"ee561cd1_89c6d5cc","line":1253,"in_reply_to":"21341812_f18c537c","updated":"2024-12-21 15:23:08.000000000","message":"AFAICT the output of `_part1()` is correlated with `c-\u003ec2.buf.len` such that `buf.len` is always \u003c\u003d 0 if `_part1()` returns false (though the reverse isn\u0027t always true). Since `_part2()` processes a packet only if `buf.len \u003e 0`, it seems safe to call `_part2()` only if `_part1()` returns true. However, as the current implementation works and I’m not entirely certain, I suggest leaving it as is unless there’s a strong reason to switch to conditional execution. What do you think?","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"},{"author":{"_account_id":1000041,"name":"ralf_lici","display_name":"Ralf Lici","email":"ralf@mandelbit.com","username":"ralf_lici"},"change_message_id":"5f0e833b85721813a9030b9b36115127ec2e3461","unresolved":false,"context_lines":[{"line_number":1250,"context_line":"            link_socket_set_outgoing_addr(lsi, \u0026c-\u003ec2.from, NULL, c-\u003ec2.es);"},{"line_number":1251,"context_line":"            tls_update_remote_addr(c-\u003ec2.tls_multi, incoming);"},{"line_number":1252,"context_line":"        }"},{"line_number":1253,"context_line":"        process_incoming_link_part2(c, lsi, orig_buf);"},{"line_number":1254,"context_line":"    }"},{"line_number":1255,"context_line":""},{"line_number":1256,"context_line":"    gc_free(\u0026gc);"}],"source_content_type":"text/x-csrc","patch_set":1,"id":"fd7d307f_f86255fe","line":1253,"in_reply_to":"ee561cd1_89c6d5cc","updated":"2025-01-07 09:53:32.000000000","message":"Done","commit_id":"6f35e2e567e3cdf87b0c95c32fe2774fee850e98"}]}
