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: Pending patches for INN? (Julien ?LIE)
2. Re: Pending patches for INN? (Richard Kettlewell)
3. Re: Pending patches for INN? (Julien ?LIE)
4. Re: Temporary dir for mailpost (Russ Allbery)
5. Re: Cast alignment warnings (Russ Allbery)
6. Re: Today's patches (Russ Allbery)
7. Re: About the development of INN 1.5.1 through 1.8 (2.0)
(Russ Allbery)
8. Re: Cast alignment warnings (Julien ?LIE)
----------------------------------------------------------------------
Message: 1
Date: Sat, 23 May 2015 14:06:26 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Cc: Richard Kettlewell <[email protected]>
Subject: Re: Pending patches for INN?
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Richard,
> I would suggest NOT delaying any release for my benefit - I devote time
> to this in very unpredictable ways.
OK, noted. Thanks again for your investment in improving
the robustness of INN.
Regarding your first patch for innxmit, shouldn't
static char *buff;
be changed to
static char *buff = NULL;
as you no longer use xmalloc, but only xrealloc?
If *buff is not initialized to NULL, it seems that it could
trigger an issue off with xrealloc trying to expand memory
from a wrong location.
As for actsync.c, maybe we could add the following test at the beginning
of get_active()?
@@ -742,7 +742,9 @@
if (len == NULL)
die("internal error #1: len is NULL");
if (errs == NULL)
- die("internal error #2: errs in NULL");
+ die("internal error #2: errs is NULL");
+ if (host == NULL)
+ die("internal error #10: host is NULL");
if (D_BUG)
warn("STATUS: obtaining active file from %s", host);
--
Julien ?LIE
? Oublie les injures, n'oublie jamais les bienfaits. ?
------------------------------
Message: 2
Date: Sat, 23 May 2015 12:31:43 +0100
From: Richard Kettlewell <[email protected]>
To: Julien ?LIE <[email protected]>, [email protected]
Subject: Re: Pending patches for INN?
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
On 2015-05-23 13:06, Julien ?LIE wrote:
> Hi Richard,
>
>> I would suggest NOT delaying any release for my benefit - I devote time
>> to this in very unpredictable ways.
>
> OK, noted. Thanks again for your investment in improving
> the robustness of INN.
No problem!
> Regarding your first patch for innxmit, shouldn't
> static char *buff;
> be changed to
> static char *buff = NULL;
> as you no longer use xmalloc, but only xrealloc?
Objects with static storage duration are initialized automatically, in
this case to a null pointer (C99 6.7.8#10) - so the two lines are
equivalent.
> If *buff is not initialized to NULL, it seems that it could
> trigger an issue off with xrealloc trying to expand memory
> from a wrong location.
>
>
>
>
> As for actsync.c, maybe we could add the following test at the beginning
> of get_active()?
>
> @@ -742,7 +742,9 @@
> if (len == NULL)
> die("internal error #1: len is NULL");
> if (errs == NULL)
> - die("internal error #2: errs in NULL");
> + die("internal error #2: errs is NULL");
> + if (host == NULL)
> + die("internal error #10: host is NULL");
> if (D_BUG)
> warn("STATUS: obtaining active file from %s", host);
Yes, that wouldn't hurt.
ttfn/rjk
------------------------------
Message: 3
Date: Sat, 23 May 2015 15:14:53 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Cc: Richard Kettlewell <[email protected]>
Subject: Re: Pending patches for INN?
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Richard,
>> Regarding your first patch for innxmit, shouldn't
>> static char *buff;
>> be changed to
>> static char *buff = NULL;
>> as you no longer use xmalloc, but only xrealloc?
>
> Objects with static storage duration are initialized automatically, in
> this case to a null pointer (C99 6.7.8#10) - so the two lines are
> equivalent.
OK, thanks for the precision.
>> As for actsync.c, maybe we could add the following test at the beginning
>> of get_active()?
>>
>> @@ -742,7 +742,9 @@
>> if (len == NULL)
>> die("internal error #1: len is NULL");
>> if (errs == NULL)
>> - die("internal error #2: errs in NULL");
>> + die("internal error #2: errs is NULL");
>> + if (host == NULL)
>> + die("internal error #10: host is NULL");
>> if (D_BUG)
>> warn("STATUS: obtaining active file from %s", host);
>
> Yes, that wouldn't hurt.
Added, then.
Thanks again.
--
Julien ?LIE
? Oublie les injures, n'oublie jamais les bienfaits. ?
------------------------------
Message: 4
Date: Sat, 23 May 2015 16:16:09 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Temporary dir for mailpost
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE <[email protected]> writes:
> Then for INN 2.6.0, we could:
> - warn in the Upgrade Section that people using mailpost without an
> explicitly configured database directory with the -b parameter should
> manually move the database from $pathtmp to $pathdb (I do not see how to
> do it automatically as we do not know how mailpost is launched; one may
> even use "-b pathtmp" explicitly);
> - change the default value for the -b option from pathtmp to pathdb;
> - add a new configurable -t option to configure the location of the
> directory to use to temporarily store error messages that are sent to the
> newsmaster. Two paths are tried by default: pathtmp as set in inn.conf,
> and then /var/tmp if pathtmp is not writable;
> - die with an explicit error message if either the directory for the
> database or the one for the temporary files refer to a path that does not
> exist or the mailpost process does not have write access to.
> And for INN 2.5.5, for backwards compatibility, just add the new -t
> option and the checks for writable directories.
> Would that sound good?
Belatedly, yup, that sounds good to me!
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
Message: 5
Date: Sat, 23 May 2015 16:23:02 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Cast alignment warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE <[email protected]> writes:
> So if I understand well, do we just need doing the following thing?
> (It builds fine, without warning.)
Yup, this patch is correct. (And it looks like you already committed it,
yay!)
> --- storage/timecaf/timecaf.c (r?vision 9831)
> +++ storage/timecaf/timecaf.c (copie de travail)
> @@ -186,15 +186,16 @@
>
> static TOKEN *PathNumToToken(char *path, ARTNUM artnum) {
> int n;
> - unsigned int t1, t2;
> + unsigned int tclass, t1, t2;
> STORAGECLASS class;
> time_t timestamp;
> static TOKEN token;
>
> - n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", (unsigned int *)&class,
> &t1, &t2);
> + n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", &tclass, &t1, &t2);
> if (n != 3)
> return (TOKEN *)NULL;
> timestamp = ((t1 << 8) & 0xff00) | ((t2 << 8) & 0xff0000) | ((t2 << 0) &
> 0xff);
> + class = tclass;
> token = MakeToken(timestamp, artnum, class, (TOKEN *)NULL);
> return &token;
> }
[...]
> I've just come across the following ones:
> buffindexed/buffindexed.c:1051:20: warning: cast from 'char *' to 'GROUPENTRY
> *'
> increases required alignment from 1 to 8 [-Wcast-align]
> GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I suspect that one could just write this as:
GROUPentries = (void *) &GROUPheader[1]
given that GROUPheader is already a pointer to a GROUPHEADER. I wonder if
that would make the warning go away. (Does GCC on amd64 produce those
warnings? I seem to recall that it doesn't.)
These are all problems with assigning the mmap'd address to a variable
that points to the first element of the structure. A better way of
representing this in C would have been to have a pointer to GROUPENTRY be
the last element of GROUPHEADER, although I'm not sure what other
assumptions that would break, somewhere else.
These alignment cast warnings are mostly because the way that mmap'd data
structures are handled are probably correct in terms of code generation
but aren't represented very well for the C type checker.
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
Message: 6
Date: Sat, 23 May 2015 16:27:14 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Today's patches
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE <[email protected]> writes:
> I have just checked how these structs are currently used, to confirm
> that it is OK to initialize all of them with 0.
> The initialization is now done this way:
> ARTHANDLE newart = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
> I hope it is fine this way (that is to say "0" instead of "NULL"
> for pointers is portable enough).
Yup, that's fine. The only place you can't use 0 (or NULL) is when
passing in a NULL pointer to a variadic function, in which case you need
to explicitly cast it to the appropriate pointer type, such as (char *) 0.
And that's because the compiler can't know the type of the argument.
Here, the compiler knows, and can translate 0 without any difficulty.
A common C convention is to define some preprocessor symbol to the
initializer for the data structure so that you can update it in only one
place if you ever change the data structure (preferrably in the same
header that defines the data structure). Like, for example,
PTHREAD_MUTEX_INITIALIZER.
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
Message: 7
Date: Sat, 23 May 2015 16:33:34 -0700
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: About the development of INN 1.5.1 through 1.8 (2.0)
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Julien ?LIE <[email protected]> writes:
> I have a question about our current SVN.
> We have tags for INN 1.5.1, INN 2.0, INN 2.1, INN 2.2.1 and all released
> versions after 2.2.1.
> I think 2.2.0 is revision 1724, according to the tar.gz, so we can
> probably add it.
That sounds like a good idea. We probably just missed making a tag at the
time.
> Do we have the 1.x versions in our SVN after 1.5.1? I see in the OLD
> section of ftp.isc.org there are 1.7.0, 1.7.1 and 1.7.2 versions after
> that one; the other versions seem to be only beta versions (1.5.2pre1
> and 1.6.0b1) so maybe INN 1.5.2 and 1.6.0 were never released.
I suspect that they were released but the tarballs were lost. We weren't
as good at keeping releases around, and things were pretty chaotic back
then. I also suspect there was no revision control in use at the time,
and that scrappy imported 1.5.1 and then imported 1.8-current when
building the initial CVS repository, but didn't have all the versions in
between. But that's just a guess; I wasn't around at the time of the
original creation of the CVS repository and am not sure.
Everything that I was able to find when I started as primary maintainer is
on the web site. More stuff is probably findable with sufficient
archeology, but I'm not sure where it would be.
> If I look for instance at the 1.7.0 tar.gz, I do not manage to match its
> CHANGES file with the changelog in our SVN. Does it mean that there
> were two different development branches at that time? One from ISC that
> was producing 1.x versions after 1.5.1 and another one in the CVS that
> became the current SVN?
There was some forking of development, and some "unofficial" versions. I
seem to recall that the 1.6 release was by... Dave Barr, maybe? And was
technically unofficial. And there was also a sort-of fork after 1.7.2
while 2.0 was under development.
--
Russ Allbery ([email protected]) <http://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<http://www.eyrie.org/~eagle/faqs/questions.html> explains why.
------------------------------
Message: 8
Date: Sun, 24 May 2015 11:26:35 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Cast alignment warnings
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Russ,
>> So if I understand well, do we just need doing the following thing?
>> (It builds fine, without warning.)
>
> Yup, this patch is correct. (And it looks like you already committed it,
> yay!)
OK, thanks for your confirmation.
I indeed went ahead after having thoroughfully tested the changes
(rebuilding my history file with makehistory, because it is the tool
that calls these modified function for articles stored in timehash and
timecaf methods).
>> I've just come across the following ones:
>
>> buffindexed/buffindexed.c:1051:20: warning: cast from 'char *' to
>> 'GROUPENTRY *'
>> increases required alignment from 1 to 8 [-Wcast-align]
>> GROUPentries = (GROUPENTRY *)((char *)GROUPheader +
>> sizeof(GROUPHEADER));
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> I suspect that one could just write this as:
>
> GROUPentries = (void *) &GROUPheader[1]
>
> given that GROUPheader is already a pointer to a GROUPHEADER. I wonder if
> that would make the warning go away. (Does GCC on amd64 produce those
> warnings? I seem to recall that it doesn't.)
amd64 does not produce those warnings. I have also just tested with the
suggested change ((void *) &GROUPheader[1]).
I found them with Clang on Mac OS X.
Should
ovblock = (OVBLOCK *)((char *)addr + pagefudge);
just be changed to
ovblock = (void *)((char *)addr + pagefudge);
in buffindexed/buffindexed.c:1667?
It suppresses the warning.
--
Julien ?LIE
? ? Vous ramassez des champignons sans les conna?tre ?
? Et alors ? Ce n'est pas pour les manger mais pour les vendre. ?
------------------------------
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
End of inn-workers Digest, Vol 72, Issue 10
*******************************************
