[hipl-dev] Re: [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: Stefan Götz <stefan.goetz@xxxxxxxxxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Mon, 15 Nov 2010 22:04:43 +0100
>> +#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'
Since it's not a very elegant solution for hiding implementation details in any
case, I'm all for merging the pairs of header files, now that their separation
causes actual problems.
>> === 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?
I would say it's okay to initialize ma to NULL. The code in this function
depends on ma never being NULL, i.e., protocol being either IPPROTO_TCP or
IPPROTO_UDP and it may seem odd that this is not tested explicitly for. But
since this is a static function, it is only called from code that checks itself
whether 'protocol' has the correct values. So I wanted to save a few cycles by
not doing the check here.
What strikes me as odd is the documentation that is terse to the point of being
useless. I'll have a look at that again.
Stefan
Other related posts:
