Irssi core bugs

Notice: Undefined index: tasklist_type in /var/www/ : eval()'d code on line 85 Notice: Undefined index: tasklist_type in /var/www/ : eval()'d code on line 90
  • Status New
  • Percent Complete
  • Task Type Feature Request
  • Category core
  • Assigned To No-one
  • Operating System Linux
  • Severity Low
  • Priority Normal
  • Reported Version irssi 0.8.13
  • Due in Version irssi 0.8.16
  • Due Date Undecided
  • Votes 0
  • Private No
Attached to Project: Irssi core bugs
Opened by Enrico Scholz (ensc) - 2009-07-26
Last edited by Wouter Coekaerts (coekie) - 2010-04-06

FS#691 - [PATCH] Proxy enhancements (incl. native SOCKS5 support)

These patches were reported more than a year ago to the maillist, but they were ignored there. While upgrading to 0.8.13 I had to rediff them against and provide them through this bugtracker.

This task does not depend on any other tasks.

Enrico Scholz (ensc)
Sunday, 26 July 2009, 14:15 GMT
PROXY/HTTP: added methods for HTTP proxies
Enrico Scholz (ensc)
Sunday, 26 July 2009, 14:16 GMT
PROXY/SIMPLE: added simple proxy method
Enrico Scholz (ensc)
Sunday, 26 July 2009, 14:16 GMT
PROXY/SOCKS5: added methods for SOCKS5 proxies
Enrico Scholz (ensc)
Sunday, 26 July 2009, 14:16 GMT
PROXY: merge proxy methods into buildsystem
Enrico Scholz (ensc)
Monday, 10 August 2009, 08:01 GMT
Wouter Coekaerts (coekie)
Tuesday, 06 April 2010, 21:25 GMT
  • Field changed: Status (Unconfirmed → New)
  • Field changed: Due in Version (Undecided → irssi 0.8.16)
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.
Enrico Scholz (ensc)
Wednesday, 07 April 2010, 08:01 GMT
* 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.
Wouter Coekaerts (coekie)
Wednesday, 14 April 2010, 18:56 GMT
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...).
Jochen Eisinger (c0ffee)
Thursday, 15 April 2010, 19:29 GMT
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?
Enrico Scholz (ensc)
Thursday, 15 April 2010, 20:57 GMT
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.
Jochen Eisinger (c0ffee)
Thursday, 15 April 2010, 23:11 GMT
it's not about what's better. if you don't intend to rewrite irssi completely, please stick with the current irssi style.
Enrico Scholz (ensc)
Saturday, 17 April 2010, 11:10 GMT
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.
Emanuele Giaquinta (ayin)
Saturday, 17 April 2010, 11:32 GMT
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).
Wouter Coekaerts (coekie)
Tuesday, 27 April 2010, 19:24 GMT
> 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.
Wouter Coekaerts (coekie)
Tuesday, 27 April 2010, 19:27 GMT
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...