Re: Patch to add netfilter mark support

On Mon, 13 Sep 2010 00:06:53 +0100, Andrew Beverley <andy_at_andybev.com>
wrote:
>> src/Parsing.*:
>> * Please palce the strtou*() functions to a file under lib/ with an .h
>> in
>> include/. At this stage they get linked in globally through
>> lib/libmisc.la.
>
> Done, but as they both return bool I've had to add stdbool.h to the
> headers that the file includes and also add a check for stdbool.h to
> configure.in. Should I stick with this methodology or is it better that
> I just change the return type to an int?

bool is fine. Your .cc must unconditionally include config.h on its first
line, stdbool is supposed to be pulled in recursively by the types
portability area. A second wrapped include in the new files does not hurt.

>> * Config.accessList.outgoingTos, Config.accessList.clientsideTos,
>> Config.accessList.outgoingNfmark, Config.accessList.clientsideNfmark
can
>> become members of the Qos scope Config object. All the parsing /free
>> stuff
>> can be moved there too with some #define parse_...() etc for the legacy
>> parser.
>
> I agree that this would be desirable, but I'm not sure how to do it
> without things getting messy. Because each of the parameters applies to
> an access list and not globally, surely they need to stay within the
> accessList struct, which means I can't move them to the Qos class?

accessList struct is just a convenient container in the old Squid
architecture for grouping all the acl related structs.

The "acl_tos *foo" field as a whole can be moved to anywhere (in this case
the QoS config class) and the cf.data.pre "LOC: Config.accessList.foo"
field + code references updated to point at the new location. This is
independent of the actual parsing changes.

Cheers
Amos
Received on Sun Sep 12 2010 - 23:58:35 MDT