[hipl-dev] Re: [Merge] lp:~hipl-core/hipl/libhip into lp:hipl
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Wed, 08 Feb 2012 19:00:42 +0100
review needs-fixing
On Wed, Feb 08, 2012 at 08:43:18AM +0000, Xin wrote:
> You have been requested to review the proposed merge of
> lp:~hipl-core/hipl/libhip into lp:hipl.
>
> === modified file 'Makefile.am'
> --- Makefile.am 2012-01-30 12:28:31 +0000
> +++ Makefile.am 2012-02-07 15:20:56 +0000
> @@ -64,19 +65,22 @@
>
> ### libraries ###
> lib_LTLIBRARIES = lib/core/libhipcore.la
> -
> +lib_LTLIBRARIES += lib/hipdaemon/libhipdaemon.la
>
> ### tests ###
Oh, the poor empty line ...
> if HIP_UNITTESTS
> -TESTS = test/check_hipd \
> - test/check_hipfw \
> - test/check_lib_core \
> - test/check_lib_tool \
> +TESTS = test/check_hipd \
> + test/check_hipfw \
> + test/check_hipnetcat \
> + test/check_lib_core \
> + test/check_lib_tool \
> test/check_modules_midauth
> -check_PROGRAMS = test/check_hipd \
> - test/check_hipfw \
> - test/check_lib_core \
> - test/check_lib_tool \
> +
> +check_PROGRAMS = test/check_hipd \
> + test/check_hipfw \
> + test/check_hipnetcat \
> + test/check_lib_core \
> + test/check_lib_tool \
> test/check_modules_midauth
> endif
Having to realign all those backslashes is suboptimal. I'll move them
all to a sensible position in trunk.
> @@ -86,55 +90,17 @@
> test_auth_performance_SOURCES = test/auth_performance.c
> test_certteststub_SOURCES = test/certteststub.c
> test_dh_performance_SOURCES = test/dh_performance.c
> -test_fw_port_bindings_performance_SOURCES =
> test/fw_port_bindings_performance.c \
> - hipfw/file_buffer.c
> \
> +test_fw_port_bindings_performance_SOURCES = hipfw/file_buffer.c
> \
> hipfw/line_parser.c
> \
> - hipfw/port_bindings.c
> + hipfw/port_bindings.c
> \
> +
> test/fw_port_bindings_performance.c
This is unrelated, push to trunk right away.
> @@ -225,6 +233,8 @@
> test/hipfw/rewrite.c \
> $(hipfw_hipfw_sources)
>
> +test_check_hipnetcat_SOURCES = test/check_hipnetcat.c
> +
> test_check_lib_core_SOURCES = test/check_lib_core.c \
> test/lib/core/crypto.c \
> test/lib/core/hit.c \
> @@ -241,21 +251,35 @@
> test/modules/midauth/hipd/midauth.c
> \
> $(hipd_hipd_sources)
>
> -
> ### static library dependencies ###
> -
Oh, those poor empty lines.
> -hipd_hipd_LDADD = lib/core/libhipcore.la
> -hipfw_hipfw_LDADD = lib/core/libhipcore.la
> -test_auth_performance_LDADD = lib/core/libhipcore.la
> -test_check_hipd_LDADD = lib/core/libhipcore.la
> -test_check_hipfw_LDADD = lib/core/libhipcore.la
> -test_check_lib_core_LDADD = lib/core/libhipcore.la
> -test_check_lib_tool_LDADD = lib/core/libhipcore.la
> -test_check_modules_midauth_LDADD = lib/core/libhipcore.la
> -test_certteststub_LDADD = lib/core/libhipcore.la
> -test_dh_performance_LDADD = lib/core/libhipcore.la
> -test_fw_port_bindings_performance_LDADD = lib/core/libhipcore.la
> -test_hc_performance_LDADD = lib/core/libhipcore.la
> +hipd_hipd_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +hipfw_hipfw_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_auth_performance_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_hipd_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_hipfw_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_hipnetcat_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_lib_core_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_lib_tool_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_check_modules_midauth_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_certteststub_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_dh_performance_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_fw_port_bindings_performance_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_hc_performance_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> +test_hipnetcat_LDADD = lib/core/libhipcore.la \
> + lib/hipdaemon/libhipdaemon.la
> tools_hipconf_LDADD = lib/core/libhipcore.la
> tools_pisacert_LDADD = lib/core/libhipcore.la
This cannot be correct. Looks like you just mechanically changed this
w/o verifying the necessity.
> --- lib/core/message.c 2011-11-08 15:25:41 +0000
> +++ lib/core/message.c 2012-02-07 15:20:56 +0000
> @@ -94,9 +94,9 @@
> #include "hip_udp.h"
> #include "icomm.h"
> #include "ife.h"
> +#include "message.h"
> #include "prefix.h"
> #include "protodefs.h"
> -#include "message.h"
unrelated and wrong
> @@ -685,6 +685,72 @@
>
> +int hip_read_control_msg_tcp(int sockfd, struct hip_packet_context *ctx)
> +{
> + HIP_IFEL(hip_verify_network_header(ctx->input_msg, &src_addr,
> + &dst_addr, len),
> + -1, "verifying network header failed\n");
> +
> +out_err:
> + return err;
> +}
HIP_IFEL abuse - I already remarked on this.
Please don't ignore my review comments. Either address all of them
before sending in a new merge request or explain why you do not address
them.
Diego
Other related posts:
