Send inn-workers mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."
Today's Topics:
1. Re: meta: Preferred patch submission policy (Julien ?LIE)
2. tradspool tooken weirdness (Christoph Biedl)
3. Re: tradspool tooken weirdness (Julien ?LIE)
4. Re: tradspool tooken weirdness (Christoph Biedl)
----------------------------------------------------------------------
Message: 1
Date: Thu, 21 Dec 2023 18:15:40 +0100
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: meta: Preferred patch submission policy
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
Hi Christoph,
> after not having been very active on this list for some time, I'd like
> to contribute to INN a bit, some patches I've created to deal better
> with the current spam issues.
Your patches are very much welcome. Thanks in advance for them.
As a side note, cleanfeed and PyClean filters may also be worthwhile
improving (but are outside the scope of INN).
> But first some meta: What's the preferred way to submit patches
> nowadays? Using this list as in the old times - or via PRs on github?
> And if the latter, on which branch should I do the work?
>
> While I can do either, I'd prefer to use this list.
Both the ways (posting to this list or via a PR) are possible.
The work should be done on the main branch. (I'll backport it on the
2.7 branch.)
--
Julien ?LIE
??Le chemin le plus court d'un point ? un autre est la ligne droite, ?
condition que les deux points soient bien en face l'un de l'autre.??
(Pierre Dac)
------------------------------
Message: 2
Date: Thu, 21 Dec 2023 20:12:56 +0100
From: Christoph Biedl <[email protected]>
To: [email protected]
Subject: tradspool tooken weirdness
Message-ID: <[email protected]>
Content-Type: text/plain; charset="us-ascii"
So I was looking into ol' tradspool, and took a peek into the history
file to see when an article is tradspooled. And eventually saw something
like this:
@05630000135A000000000000628500000000@
Even without checking any documentation it was pretty obivous: Storage
method 5, class 99, group number 0x135a = 4954, article 0x6285 = 25221;
and yes, that's it.
However, this does not match the token description as in tradspool.c:
** The token is @05nnxxxxxxxxyyyyyyyy0000000000000000@
** where "05" is the tradspool method number,
** "nn" the hexadecimal value of the storage class,
** "xxxxxxxx" the name of the primary newsgroup (as defined
** in <pathspool>/tradspool.map),
** "yyyyyyyy" the article number in the primary newsgroup.
More precisely, the article number part should be eight positions
further to the left. And indeed, "sm -c" gets it wrong, check artnum in
the output:
@05630000135A000000000000628500000000@ \
method=tradspool class=99 ngnum=4954 artnum=0 \
file=(...)/spool/articles/comp/lang/mumps/25221
Before you freak out, the only real problem is the visual in "sm -c"
above.
So, this is on amd64, and I'm certain this happens on all 64 bit
architentures, or more precisely: Where sizeof(long) is 8 - but I guess
that's the same thing.
The underlying issue is the assumption sizeof(uint32_t) == sizeof(long)
which holds for 32 bit. But here, MakeToken gets an article number
(artnum) as unsigned long (64 bit), converts it using htonl which is 32
bit, likewise for (group) num. For storing the article number however ,
sizeof(num) is used as the offset, so 8, while by design it should be
only 4.
Luckily, the reverse operation TokenToPath uses the same semantics, so
things do work as expected, and nobody noticed in decades. Only
tradspool_explaintoken fails since it uses uint32_t which follows the
design but does not match the implementation.
There are several ways to get out of this. Quick and dirty, adjust
tradspool_explaintoken and the documentation. Nicer was to make the
implementation follow the design, replace unsigned long with uint32_t
when it's about article and group numbers[1], and have a transition in
the lookup code for the article number: It would check both offsets (4
and 8), the non-zero one has the actual information. That might as
well be done as part of an upgrade.
Cheers,
Christoph
[1] Perhaps some day there'll be an architecture/ABI where long is
128 bit. Then we're in trouble.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.isc.org/pipermail/inn-workers/attachments/20231221/a97a1a16/attachment-0001.sig>
------------------------------
Message: 3
Date: Thu, 21 Dec 2023 20:58:22 +0100
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: tradspool tooken weirdness
Message-ID: <[email protected]>
Content-Type: text/plain; charset=UTF-8; format=flowed
Hi Christoph,
> So I was looking into ol' tradspool, and took a peek into the history
> file to see when an article is tradspooled. And eventually saw something
> like this:
>
> @05630000135A000000000000628500000000@
>
> Even without checking any documentation it was pretty obivous: Storage
> method 5, class 99, group number 0x135a = 4954, article 0x6285 = 25221;
> and yes, that's it.
>
> However, this does not match the token description as in tradspool.c:
>
> ** The token is @05nnxxxxxxxxyyyyyyyy0000000000000000@
> ** where "05" is the tradspool method number,
> ** "nn" the hexadecimal value of the storage class,
> ** "xxxxxxxx" the name of the primary newsgroup (as defined
> ** in <pathspool>/tradspool.map),
> ** "yyyyyyyy" the article number in the primary newsgroup.
>
> More precisely, the article number part should be eight positions
> further to the left. And indeed, "sm -c" gets it wrong, check artnum in
> the output:
>
> @05630000135A000000000000628500000000@ \
> method=tradspool class=99 ngnum=4954 artnum=0 \
> file=(...)/spool/articles/comp/lang/mumps/25221
>
> Before you freak out, the only real problem is the visual in "sm -c"
> above.
>
>
> So, this is on amd64, and I'm certain this happens on all 64 bit
> architentures, or more precisely: Where sizeof(long) is 8 - but I guess
> that's the same thing.
Yes, I also have the same syntax you see for tokens on my amd64 server
with 8 bytes long.
> There are several ways to get out of this. Quick and dirty, adjust
> tradspool_explaintoken and the documentation.
This would be the easiest and first thing to do.
Sorry for not having seen that. When I wrote the
tradspool_explaintoken() function, I was running a 32-bit OS and did not
spot that.
> Nicer was to make the
> implementation follow the design, replace unsigned long with uint32_t
> when it's about article and group numbers[1], and have a transition in
> the lookup code for the article number. It would check both offsets (4
> and 8), the non-zero one has the actual information. That might as
> well be done as part of an upgrade.
Why would there be a need for a transition?
On a given server, there won't be mixed tokens (ones with 32-bit long
numbers and others with 64-bit long numbers), and the history file needs
rebuilding when doing such a change of architecture or
enabling/disabling largefile support.
When would we have a case of mixed tokens like that in a history file,
and "sm -c" failing to decode some of them?
--
Julien ?LIE
???tre ?u ne p?s ?tre, telle est l? questi?n??? (Ker?zen)
------------------------------
Message: 4
Date: Thu, 21 Dec 2023 23:44:56 +0100
From: Christoph Biedl <[email protected]>
To: [email protected]
Subject: Re: tradspool tooken weirdness
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE wrote...
> Hi Christoph,
>
> > Nicer was to make the
> > implementation follow the design, replace unsigned long with uint32_t
> > when it's about article and group numbers[1], and have a transition in
> > the lookup code for the article number. It would check both offsets (4
> > and 8), the non-zero one has the actual information. That might as
> > well be done as part of an upgrade.
>
> Why would there be a need for a transition?
> On a given server, there won't be mixed tokens (ones with 32-bit long
> numbers and others with 64-bit long numbers), and the history file needs
> rebuilding when doing such a change of architecture or enabling/disabling
> largefile support.
> When would we have a case of mixed tokens like that in a history file, and
> "sm -c" failing to decode some of them?
No architecture switch, all this was within amd64 (or other 64 bit
archs).
Assuming INN was modfied to create and parse the tokens as specified.
Then the upgrade brings a problem as the existing tradspool tokens
become invalid.
To deal with this, I can think of
* Big warning in the release notes a history rebuild is needed during
upgrade when using tradspool
Pro: Not much work
Con: Hazzle for the users, some will forget this and be annoyed
afterwards.
* Extra code that deals with these invalid tokens by picking the article
number from the place where it is. That the transition I was talking
about.
* Additionally, the next news.daily could repair these invalid tokens as
well.
Pro: The handling of invalid tokens could be removed again faily soon,
say after the next major release.
Hope that makes my thoughts a bit more clear. I prefer clean solutions
even if they mean more work (here: Create tokens as specified) over
quick hacks (which give me a bad feeling for all the time that follows).
Christoph
------------------------------
Subject: Digest Footer
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
End of inn-workers Digest, Vol 156, Issue 4
*******************************************
