[hipl-dev] Re: [Branch ~hipl-core/hipl/trunk] Rev 5151: Merge firewall cache port cleanup branch from Stefan Götz; minor fixes from me.
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Mon, 15 Nov 2010 20:22:33 +0100
On Mon, Nov 15, 2010 at 12:43:34PM +0000, noreply@xxxxxxxxxxxxx wrote:
> Merge authors:
> goetz <goetz@goetz>
> Stefan Götz (stefan.goetz)
> Related merge proposals:
>
> https://code.launchpad.net/~stefan.goetz/hipl/fw-cache-port-cleanup/+merge/39235
> proposed by: Stefan Götz (stefan.goetz)
> review: Approve - Diego Biurrun (diego-biurrun)
> review: Approve - Stefan Götz (stefan.goetz)
> ------------------------------------------------------------
> revno: 5151 [merge]
> committer: Diego Biurrun <diego@xxxxxxxxxx>
> branch nick: trunk
> timestamp: Fri 2010-11-12 18:49:42 +0100
> message:
> Merge firewall cache port cleanup branch from Stefan Götz; minor fixes from
> me.
> added:
> firewall/file_buffer.h
> firewall/file_buffer_inline.h
> firewall/line_parser.h
> firewall/line_parser_inline.h
> renamed:
> firewall/cache_port.c => firewall/port_bindings.c
> firewall/cache_port.h => firewall/port_bindings.h
>
> --- firewall/file_buffer.h 1970-01-01 00:00:00 +0000
> +++ firewall/file_buffer.h 2010-11-12 17:49:42 +0000
> @@ -0,0 +1,46 @@
> +
> +#ifndef HIP_FIREWALL_FILE_BUFFER_H
> +#define HIP_FIREWALL_FILE_BUFFER_H
> +
> +#include "mem_area.h"
> +
> +struct hip_file_buffer;
> +
> +int hip_fb_create(struct hip_file_buffer *const fb,
> + const char *const file_name);
> +void hip_fb_delete(struct hip_file_buffer *const fb);
> +static inline const struct hip_mem_area *hip_fb_get_mem_area(const struct
> hip_file_buffer *const fb);
> +int hip_fb_reload(struct hip_file_buffer *const fb);
> +
> +#include "firewall/file_buffer_inline.h"
> +
> +#endif /* HIP_FIREWALL_FILE_BUFFER_H */
>
> --- firewall/file_buffer_inline.h 1970-01-01 00:00:00 +0000
> +++ firewall/file_buffer_inline.h 2010-11-12 17:49:42 +0000
> @@ -0,0 +1,98 @@
> +
> +#ifndef HIP_FIREWALL_FILE_BUFFER_INLINE_H
> +#define HIP_FIREWALL_FILE_BUFFER_INLINE_H
> +
> +/* On the one hand, the contents of this file are part of the public
> interface
> + * and thus only their declaration should go into the public header file.
> + * On the other hand, these functions should be inlineable so their
> definitions
> + * have to appear in a header file.
> + * To achieve inlineability and still hide the implementation, we use this
> + * secondary header file that is not part of the public interface. */
> +#ifndef HIP_FIREWALL_FILE_BUFFER_H
> +#error This file must not be included directly because it contains
> implementation details. It may only be included by file_buffer.h.
> +#endif
This (and the similar change in line_parser.h/line_parser_inline.h) breaks
'make checkheaders':
~/src/hipl/vanilla $ make -k checkheaders
gcc -I. -I. -D_POSIX_C_SOURCE=200112L -D_XOPEN_SOURCE=500 -std=c99 -Wall
-Wextra -Werror -Wredundant-decls -Wdisabled-optimization -Wundef
-Wstrict-prototypes -Wmissing-prototypes -Wno-deprecated-declarations
-Wpointer-arith -Wwrite-strings -Wshadow -Winline -Wcast-qual
-fno-strict-aliasing -g -O2 -o firewall/file_buffer_inline.ho
firewall/file_buffer_inline.h
firewall/file_buffer_inline.h:41: error: #error This file must not be included
directly because it contains implementation details. It may only be included by
file_buffer.h.
make: *** [firewall/file_buffer_inline.ho] Error 1
gcc -I. -I. -D_POSIX_C_SOURCE=200112L -D_XOPEN_SOURCE=500 -std=c99 -Wall
-Wextra -Werror -Wredundant-decls -Wdisabled-optimization -Wundef
-Wstrict-prototypes -Wmissing-prototypes -Wno-deprecated-declarations
-Wpointer-arith -Wwrite-strings -Wshadow -Winline -Wcast-qual
-fno-strict-aliasing -g -O2 -o firewall/line_parser_inline.ho
firewall/line_parser_inline.h
firewall/line_parser_inline.h:41: error: #error This file must not be included
directly because it contains implementation details. It may only be included by
line_parser.h.
make: *** [firewall/line_parser_inline.ho] Error 1
make: Target `checkheaders' not remade because of errors.
That test checks that all headers can be compiled standalone, while the
_inline.h headers are explicitly designed not to work standalone. We
cannot have both at the same time obviously, so we must decide between
them.
I am tempted to merge the _inline.h headers into their non-inline
counterparts. I know that this undoes some heroic attempts at API
abuse prevention, but I think the chance for that happening is low
and in the meantime, 'make checkheaders' provides a useful service.
Opinions?
> === renamed file 'firewall/cache_port.c' => 'firewall/port_bindings.c'
> --- firewall/cache_port.c 2010-10-15 15:29:14 +0000
> +++ firewall/port_bindings.c 2010-11-12 17:49:42 +0000
> @@ -25,277 +25,465 @@
>
> /**
> + * Look up the port binding from the proc file system.
> + *
> + * @param protocol protocol type
> + * @param port the port number of the socket
> + * @return the traffic type associated with the given port.
> */
> +static enum hip_port_binding hip_port_bindings_get_from_proc(const uint8_t
> protocol,
> + const uint16_t
> port)
> {
> + const unsigned int PORT_STR_OFFSET = 39;
> + const unsigned int PORT_STR_LEN = 4;
> + enum hip_port_binding result = HIP_PORT_INFO_IPV6UNBOUND;
> + const struct hip_mem_area *ma;
> + char *line;
> + // the files /proc/net/{udp,tcp}6 are line-based and the line number of
> the
> + // port to look up is not known in advance
> + // -> use a parser that lets us iterate over the lines in the files
> + struct hip_line_parser lp;
> +
> + switch (protocol) {
> + case IPPROTO_TCP:
> + ma = hip_fb_get_mem_area(&tcp6_file);
> + break;
> case IPPROTO_UDP:
> + ma = hip_fb_get_mem_area(&udp6_file);
> + break;
> + }
> + hip_lp_create(&lp, ma);
This causes the following warning (and thus error) on
CentOS 5.5 with gcc 4.1.2:
firewall/port_bindings.c: In function ‘hip_port_bindings_get’:
firewall/port_bindings.c:315: warning: ‘ma’ may be used uninitialized in this
function
I'm not sure what the best way to fix this would be. Stefan?
Diego
Other related posts:
