Irssi core bugs

PROXY/HTTP: added methods for HTTP proxies

PROXY/SIMPLE: added simple proxy method

PROXY/SOCKS5: added methods for SOCKS5 proxies

PROXY: merge proxy methods into buildsystem

Now managed at http://github.com/ensc/irssi-proxy/tree/master

This is on the list of things we should really look at before 0.8.16.

I've only taken a very quick look at the patches. The functionality it provides certainly seems useful.

I've got my doubts (but am still undecided) about the "container_of" usage, mostly because it seems inconsistent with how such things are handled elsewhere in irssi code.

* they provide not only new functionality, they fix existing one too (e.g. current HTTP proxy does not work when proxy does not accept data before the "200 connected" response)

* container_of() is common style e.g. in the linux kernel and is used for class inheritance. Without it, you would have to add a private data pointer to each class, and would have to handle two memory allocations (one for the specialised class, one for the base one) where container_of() requires only one.

We've got inheritance without two memory allocations in irssi too (see *-rec.h). I'm not saying that's a _better_ mechanism (I'd even say it definitely isn't), but I'm not sure we want to mix (and am hoping on input from other irssi devs...).

Sorry I didn't see this patch earlier... a quick search returned #477. Please link bugs and their patches in the future.

Anyway, I've implemented basic http proxy support like this (see attachment)

I see your patch supports more protocols. However, it is not irssi style, so I guess it needs to be rewritten.

Any comments on what to do?

What is the idea behind this -rec.h stuff? Is it there to support compilers which do not understand nested struct's?

On first glance its seems to be very unportable; e.g. in

--- base-rec.h ---
char x;
char y;
int z;

--- foo.h ---
struct base {
#include "base-rec.h"
};

struct foo {
char a;
#include "base-rec.h"
};

--

foo::z and base::z will be on different positions relative to the ::x attribute. This means, when you cast the 'base-rec' object in 'foo' to 'base' you will access bad memory.

it's not about what's better. if you don't intend to rewrite irssi completely, please stick with the current irssi style.

I do not want to rewrite irssi completely; but I don't want to provide hacky and unportable code (which begins at identifiers like __PROXY_H or _SERVER_CONNECT_REC which are forbidden/reserved by the C standard and continues with random attribute location due to inheritance-by-preprocessor hack) in new contributions.

Because I am using irssi, I am basically willing to rewrite the whole -rec stuff in a portable way. But my experiences with this patchset shows, that I might have to wait other two years until I get a single comment about these changes. As they will touch a lot of code, it will be (too) much work to keep them in sync with upstream.

That would be a welcome contribution I think, but note that your current code is not portable either (you use __type_of__ and __attribute__ unconditionally).

> struct foo {
> char a;
> #include "base-rec.h"

Irssi doesn't do it like that, the include is always the first thing in the struct.

I looked at the patches again, considering porting them to irssi-style... but a bigger problem with them is that it is doing synchronous io. That's not so easily fixed...