On Mon, Feb 13, 2012, at 23:16, Juliusz Chroboczek wrote:
> >  * automated-testing introduces systemd socket activation support
> 
> No way, sorry.  Polipo can be reliably restarted by systemd (and runit,
> and daemontools, and V7 init) without the need to include 600 lines of
> code that I don't understand.

For clarity, systemd support was added for socket activation rather than
process supervision.  Indeed we don't actually use systemd here, but we
require the ability to pass a socket to polipo so we can create a
private sandboxed HTTP cache, per application, on our embedded set-top
Linux without having to wait for polipo to start up to query it for
which port it has opened on.  We chose to make it systemd compatible as
we thought that that might be more useful to others.

I agree that adding 600 lines of sd_daemon.[ch] is a little bit silly
considering that the bits that are used are just checking that
getenv("LISTEN_FDS") == "1" and getenv("LISTEN_PID") = getpid().  I will
rewrite the patch set to not include those files.

> >    simple automated test framework.
> 
> Ah... that could be interesting.

It proved invaluable for tracking down the "Polipo always revalidates
resources with Content-Length >= CHUNK_SIZE" bug.

> >  * fix-flags cleans up the naming of the OBJECT_TYPE, OBJECT_FLAG and
> >    CACHE_CONTROL_FLAG values
> 
> No, sorry.  It makes the code uselessly verbose for no gain.

I disagree that it is for no gain.  It was precisely this change that
made the typo in httpTweakCachability obvious.  Without the change the
code reads:

    if((object->cache_control & CACHE_AUTHORIZATION) &&
       !(object->cache_control & CACHE_PUBLIC))
        object->cache_control |= (CACHE_NO_HIDDEN | OBJECT_LINEAR);

This possibly looks suspicious, but not immediately broken.

And with it:

    if((object->cache_control & CACHE_CONTROL_FLAG_AUTHORIZATION) &&
       !(object->cache_control & CACHE_CONTROL_FLAG_PUBLIC))
        object->cache_control |= (CACHE_CONTROL_FLAG_NO_HIDDEN |
        OBJECT_FLAG_LINEAR);

The second is at a glance obviously incorrect because CACHE_CONTROL_FLAG
matches cache_control so OBJECT_FLAG stands out like a sore thumb.  This
change has already helped to fix bugs.

I think an even stronger case can be made for making object type an enum
as this allows the compiler to check for proper use and warn/error if
incorrectly used.

Thanks

Will

------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
Polipo-users mailing list
Polipo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/polipo-users

Reply via email to