Julien ÉLIE | 5 Jun 10:15 2010

Re: [PATCH] Improve NNTPconnect error reporting

Hi Ian,

> In inn2 2.4.5, NNTPconnect does not always report errors very well.

I see that INN 2.5.2 has a similar behaviour.

> In the patch below I address this by always arranging for the buffer
> to contain a useful message.

The problem is that I cannot apply the patch.  The whole code has changed
in INN 2.5.2.

>  <at>  <at>  -53,8 +64,11  <at>  <at>  int NNTPconnect(char *host, int port, FILE **FromServerp, FILE **ToServerp, char
>     hints.ai_family = PF_UNSPEC;
>     hints.ai_socktype = SOCK_STREAM;
>     sprintf(portbuf, "%d", port);
> -    if (getaddrinfo(host, portbuf, &hints, &addr) != 0)
> +    if ((gai_r = getaddrinfo(host, portbuf, &hints, &addr)) != 0) {
> + store_error(buff, "getaddrinfo failed: ", gai_strerror(gai_r));
> + errno = 0;
>  return -1;
> +    }
>
>     for (ressave = addr; addr; addr = addr->ai_next) {
>  if ((i = socket(addr->ai_family, addr->ai_socktype,

It is now driven in network_connect_host() in lib/network.c

/*
**  Like network_connect, but takes a host and a port instead of an addrinfo
(Continue reading)

Julien ÉLIE | 5 Jun 10:38 2010

Re: allow 'cat' as log compression method

Hi Florian,

> I wonder if there's anybody else who'd prefer to *not* compress his logs
> in any way (and rather not keep it for too long to save space)?

Thanks for the idea.
Just committed.

> This is --with-log-compress=cat (I didn't bother to go hunting for cat
> as it's part of coreutils - should we do that?):

I added the check so that LOG_COMPRESS is properly set.
It seems fine in my config.log if I configure INN to use "cat":

LOG_COMPRESS='/bin/cat'
LOG_COMPRESSEXT=''

-dnl compress instead and others may want to use bzip2.  INN also needs to
-dnl locate gzip regardless, since it's used for compressed rnews batches, and
-dnl needs to know how to uncompress .Z files.
+dnl compress instead, and others may want to use bzip2, or even not compress
+dnl at all their logs.  INN also needs to locate gzip regardless, since it's
+dnl used for compressed rnews batches, and needs to know how to uncompress .Z
+dnl files.

and afterwards:

+AC_ARG_VAR([CAT], [Location of cat program])
+AC_PATH_PROG([CAT], [cat], [cat])

(Continue reading)

Ian Jackson | 5 Jun 15:14 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Julien ÉLIE writes ("Re: [PATCH] Improve NNTPconnect error reporting"):
> I see that INN 2.5.2 has a similar behaviour.

Right ...

> The problem is that I cannot apply the patch.  The whole code has changed
> in INN 2.5.2.

Hrm.  I will take a look at 2.5.2 and see what I think needs to be
done.

Thanks,
Ian.
Russ Allbery | 5 Jun 18:37 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Julien ÉLIE <julien <at> trigofacile.com> writes:

> It is now driven in network_connect_host() in lib/network.c

> /*
> **  Like network_connect, but takes a host and a port instead of an addrinfo
> **  struct list.  Returns the file descriptor of the open socket on success,
> **  or -1 on failure.  If getaddrinfo fails, errno may not be set to anything
> **  useful.
> */

> and not directly in lib/remopen.c where we have:

>    fd = network_connect_host(host, port, NULL);
>    if (fd < 0)
>        return -1;

> with no information from the getaddrinfo return code.
> The network API (headers in include/inn/network.h) would then need to be
> changed if we have to pass it a buffer to write the error.
> Is it advisable to do that?

It might be.  That was always a really ugly wart in the network API and I
was cringing a bit when I wrote that last sentence in the introductory
comment.  It's frustrating that there are two entirely separate error
reporting mechanisms between the system calls and the DNS lookups that
complicate passing errors back.

I think adding a buffer to hold the error is kind of ugly, but if it's the
only way to get the real error out, it might be worth it.  I suppose we
(Continue reading)

Ian Jackson | 7 Jun 00:09 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Russ Allbery writes ("Re: [PATCH] Improve NNTPconnect error reporting"):
> It might be.  That was always a really ugly wart in the network API and I
> was cringing a bit when I wrote that last sentence in the introductory
> comment.  It's frustrating that there are two entirely separate error
> reporting mechanisms between the system calls and the DNS lookups that
> complicate passing errors back.

Also, with any connection routine like this that tries multiple
addresses and address families, there's a question about which errno
value to report, which leads to slightly obscure logic about saving
and restoring errnos.  Another possibility would be to allow the
caller to specify a callback function which gets told either a string,
or some more information including h_errno or errno or what have you,
and might be called more than once.

> I think adding a buffer to hold the error is kind of ugly, but if it's the
> only way to get the real error out, it might be worth it.  I suppose we
> could do something sneaky like return both positive and negative error
> codes and distinguish between errno and h_errno that way, but that's
> almost as ugly.

I think the string buffer is probably least ugly but tell me what you
think is best and I'll code it up.

Ian.
Russ Allbery | 7 Jun 03:38 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Ian Jackson <ijackson <at> chiark.greenend.org.uk> writes:
> Russ Allbery writes ("Re: [PATCH] Improve NNTPconnect error reporting"):

>> It might be.  That was always a really ugly wart in the network API and
>> I was cringing a bit when I wrote that last sentence in the
>> introductory comment.  It's frustrating that there are two entirely
>> separate error reporting mechanisms between the system calls and the
>> DNS lookups that complicate passing errors back.

> Also, with any connection routine like this that tries multiple
> addresses and address families, there's a question about which errno
> value to report, which leads to slightly obscure logic about saving and
> restoring errnos.

Yeah, also true.  I'm sure the current code doesn't do that properly.

> Another possibility would be to allow the caller to specify a callback
> function which gets told either a string, or some more information
> including h_errno or errno or what have you, and might be called more
> than once.

That would at least be the most complete solution, although it might be
overkill.

>> I think adding a buffer to hold the error is kind of ugly, but if it's
>> the only way to get the real error out, it might be worth it.  I
>> suppose we could do something sneaky like return both positive and
>> negative error codes and distinguish between errno and h_errno that
>> way, but that's almost as ugly.

(Continue reading)

Ian Jackson | 7 Jun 12:52 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Russ Allbery writes ("Re: [PATCH] Improve NNTPconnect error reporting"):
> A string buffer seems to be about the right level of complexity, at
> least.  The only thing that makes me leery of it is that there's no good
> way to size that buffer; one just has to take a guess at what would be big
> enough.

Yes.  But OTOH the whole thing is already full of fixed-size buffers
so we wouldn't be making it much worse :-).

> On UNIX, you can probably get away with passing in a char ** and
> allocating the error buffer on the fly, but I'd rather not go that route
> just in case the INN version of this code is ever reunified with the
> version that I use on other projects, since that breaks on Windows without
> adding a corresponding function to free the error buffer.

That would be acceptable I guess.  But are we allowed to use asprintf ?
If not then printing into an allocated buffer is going to be such a
pain we'd probably rather not.

Ian.
(writing without the source to hand)
Russ Allbery | 7 Jun 19:08 2010
Picon

Re: [PATCH] Improve NNTPconnect error reporting

Ian Jackson <ijackson <at> chiark.greenend.org.uk> writes:
> Russ Allbery writes ("Re: [PATCH] Improve NNTPconnect error reporting"):

>> A string buffer seems to be about the right level of complexity, at
>> least.  The only thing that makes me leery of it is that there's no good
>> way to size that buffer; one just has to take a guess at what would be big
>> enough.

> Yes.  But OTOH the whole thing is already full of fixed-size buffers
> so we wouldn't be making it much worse :-).

Well, NNTPconnect is, indeed, but network isn't.  There are parts of INN's
code that are kind of ugly and parts that are newer and cleaner, and I try
to keep the ugly bits from infecting the newer bits until we can rewrite
the ugly bits.  :)

>> On UNIX, you can probably get away with passing in a char ** and
>> allocating the error buffer on the fly, but I'd rather not go that
>> route just in case the INN version of this code is ever reunified with
>> the version that I use on other projects, since that breaks on Windows
>> without adding a corresponding function to free the error buffer.

> That would be acceptable I guess.  But are we allowed to use asprintf ?

Yes, INN provides asprintf if it's not available.  And the more I think
about it, the more it may make sense to just use asprintf and then provide
a separate function to free the error string.

Proper error handling is always the hardest part of writing in C.

(Continue reading)

The Doctor | 8 Jun 18:24 2010
Picon

DB 5.0

Anyone knows if this would work with INN 2.X ?

--

-- 
Member - Liberal International	This is doctor <at> nl2k.ab.ca Ici doctor <at> nl2k.ab.ca
God, Queen and country! Never Satan President Republic! Beware AntiChrist rising! 
http://twitter.com/rootnl2k http://www.facebook.com/dyadallee
Since 1 June 1995
William F. Maton Sotomayor | 15 Jun 21:27 2010
Picon

Re: [apps-discuss] Publication of NNTP extensions on the Standards Tracks (fwd)


FYI

wfms

---------- Forwarded message ----------
Date: Tue, 15 Jun 2010 12:20:58 -0700
From: Murray S. Kucherawy <msk <at> cloudmark.com>
To: Alexey Melnikov <alexey.melnikov <at> isode.com>
Cc: "Apps-Discuss <at> ietf.org" <Apps-Discuss <at> ietf.org>
Subject: Re: [apps-discuss] Publication of NNTP extensions on the Standards
     Tracks

> -----Original Message-----
> From: Alexey Melnikov [mailto:alexey.melnikov <at> isode.com]
> Sent: Tuesday, June 15, 2010 12:11 PM
> To: Murray S. Kucherawy
> Cc: Apps-Discuss <at> ietf.org
> Subject: Re: [apps-discuss] Publication of NNTP extensions on the Standards Tracks
>
> I am concerned about the number of users of the technology in general
> and experts/reviewers in particular. Some Area Directors have much
> higher bar for sponsoring documents than I do, yet I am still trying to
> decide if investing my AD time in NNTP is a good way of spending time.

Unfortunately I don't think NNTP specs fit into another area.  And my understanding is that a
non-experimental specification defining a protocol that traverses ADMD boundaries has to be standards track.

But the issue of the absence of expertise in the IETF to conduct meaningful reviews is an interesting one, as
is the presumably dwindling user base.  Perhaps those are reasons to do them as informational or
(Continue reading)


Gmane