[squid-dev] [PATCH] OSX transparent-proxy using pfctl

[Alex Rousskov]

On 09/29/2016 01:12 PM, Shively, Gregory wrote:

> Sometimes these mailing lists make me think like I'm talking to one
> person :-).

Glad we all sound coherent to you :-)!


> ERROR: files left in build directory after distclean:
> ./src/cf_gen.dSYM/Contents/Info.plist
> ./src/cf_gen.dSYM/Contents/Resources/DWARF/cf_gen

> I can run with the "--keep-going" flag, but that ends with a the
> equivalent error in btlayer-05-no-deps-esi. Do I run it this way and
> just grep the logs for FAIL or something?

Sounds like a good plan to me.


> I wasn't sure if I should handle it or let it flow up, since if it
> was in an overflow state I would doubt I could handle this packet,
> but maybe the next connection would be successful.

I recommend temporary adding an exception with Must(false) inside the
parsing code and testing what happens. This approach may give you enough
information to decide whether your code or the caller should be
responsible for handling parsing exceptions.


> +    while ((n = read(pipefd[0], buf, sizeof(buf))) > 0) {
> +        state.append(buf);

You need to tell SBuf how much to append(). read(2) does not 0-terminate
buf!


> +    if (n == -1) {
> +        int xerrno = errno;

Too late: errno may already be different after close().


> +        // Move to EOL and try again from the next line
> +        tk.token(host, lf);

If possible, use one of the skip() methods to avoid the implication that
you are parsing host here (and to save a couple of CPU cycles).

Please note that Tokenizer has a method to extract integers. I am not
sure it will help you here, but it might.

> +                newConn->local = host.c_str();
> +                newConn->local.port(atol(port.c_str()));
> +                debugs(89, 5, HERE << "address NAT: " << newConn);
> +                return true;

> +                newConn->local = host.c_str();
> +                newConn->local.port(atol(port.c_str()));
> +                debugs(89, 5, HERE << "address NAT: " << newConn);

If you can avoid code duplication, please do so.

> +    CharacterSet bracket("bracket", "[]");
> +    CharacterSet colon("colon", ":");
> +    CharacterSet lf("lf", "\n");
> +    CharacterSet ws("ws", " \t\r\n");

These should be declared as static to avoid allocating/initializing lots
of bytes on every access and as constant to help the optimizer (and to
avoid future bugs). And you can move them inside the loop then -- they
are unused outside the loop.

In general, declare variables as late as possible and make them const if
possible.


Thank you,

Alex.