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. Tradindexed with non ASCII newsgroup names (Julien ?LIE)
2. Re: Cast alignment warnings (Julien ?LIE)
3. Re: Today's patches (Julien ?LIE)
4. RE: Today's patches (Matt Seitz (matseitz))
5. Re: Today's patches (Julien ?LIE)
6. Re: Today's patches (Julien ?LIE)
7. RE: Today's patches (Matt Seitz (matseitz))
----------------------------------------------------------------------
Message: 1
Date: Fri, 08 May 2015 15:13:33 +0200
From: Julien ?LIE <[email protected]>
To: "[email protected]" <[email protected]>
Subject: Tradindexed with non ASCII newsgroup names
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi all,
Though tradindexed overview technically works with international
newsgroup names (UTF-8), one should note the following thing:
~/spool/overview/t/t% find .
./trigofacile.test.IDX
./trigofacile.test.DAT
./v
./v/trigofacile.test.vide.IDX
./v/trigofacile.test.vide.DAT
./?
./?/trigofacile.test.???8.IDX
./?/trigofacile.test.???8.DAT
./?
./?/trigofacile.test.?.IDX
./?/trigofacile.test.?.DAT
./?/trigofacile.test.??.IDX
./?/trigofacile.test.??.DAT
./?/trigofacile.test.??.IDX
./?/trigofacile.test.??.DAT
./?
./?/trigofacile.test.????.IDX
./?/trigofacile.test.????.DAT
The "?" chars in my terminal are: '\316', '\317' and '\341'.
The directory structure is made with the first byte of each component.
Thus, for instance trigofacile.test.???8 is not in spool/overview/t/t/?
(upsilon).
Finding a newsgroup in such a structure will be harder for a human.
Maybe we should keep it as-is (otherwise, performance will probably be
affected if we have to first extract the first UTF-8 character of each
component).
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
------------------------------
Message: 2
Date: Fri, 08 May 2015 17:28:51 +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
Hi Russ,
Following up an old thread:
>> We have in timecaf/timehash an unsigned char class that is casted to an
>> unsigned int* for the sake of sscanf().
>>
>> n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", (unsigned int*)&class,
>> &t1, &t2);
>
> Actually, I think that one *is* a problem; the address of class is not
> guaranteed to be aligned, since it's a char, which means that sscanf may
> do an unaligned store of an integer. It's also wrong for other reasons:
> integers are probably either four or eight bytes, so sscanf is going to
> write at least four bytes at the address of class, which is going to
> blithely overwrite neighboring variables on the stack.
>
> That looks like a real bug. The code needs to declare a temporary
> unsigned int variable, sscanf into it, and then store the results in
> class.
So if I understand well, do we just need doing the following thing?
(It builds fine, without warning.)
--- 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;
}
--- storage/timehash/timehash.c (r?vision 9831)
+++ storage/timehash/timehash.c (copie de travail)
@@ -114,16 +114,17 @@
static TOKEN *PathToToken(char *path) {
int n;
- unsigned int t1, t2, t3, seqnum;
+ unsigned int tclass, t1, t2, t3, seqnum;
STORAGECLASS class;
time_t now;
static TOKEN token;
n = sscanf(path, "time-%02x/%02x/%02x/%04x-%04x",
- (unsigned int *)&class, &t1, &t2, &seqnum, &t3);
+ &tclass, &t1, &t2, &seqnum, &t3);
if (n != 5)
return (TOKEN *)NULL;
now = ((t1 << 16) & 0xff0000) | ((t2 << 8) & 0xff00) | ((t3 << 16) &
0xff000000) | (t3 & 0xff);
+ class = tclass;
token = MakeToken(now, seqnum, class, (TOKEN *)NULL);
return &token;
}
--- nnrpd/sasl.c (r?vision 9831)
+++ nnrpd/sasl.c (copie de travail)
@@ -115,6 +115,7 @@
const char *mech;
const char *clientin = NULL;
unsigned int clientinlen = 0;
+ size_t tclientinlen = 0;
const char *serverout = NULL;
unsigned int serveroutlen;
char base64[BASE64_BUF_SIZE+1];
@@ -209,7 +210,8 @@
/* Get the response from the client. */
r1 = line_read(&NNTPline, PERMaccessconf->clienttimeout,
- &clientin, (size_t *) &clientinlen, NULL);
+ &clientin, &tclientinlen, NULL);
+ clientinlen = tclientinlen;
switch (r1) {
case RTok:
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));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1206:18: warning: cast from 'char *' to 'GROUPENTRY *'
increases required alignment from 1 to 8 [-Wcast-align]
GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1241:18: warning: cast from 'char *' to 'GROUPENTRY *'
increases required alignment from 1 to 8 [-Wcast-align]
GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1667:15: warning: cast from 'char *' to 'OVBLOCK *'
(aka 'struct _OVBLOCK *') increases required alignment from 1 to 8
[-Wcast-align]
ovblock = (OVBLOCK *)((char *)addr + pagefudge);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Wouldn't it be enough for them to only add a cast to (void *) just before
we cast to (GROUPENTRY *) or (OVBLOCK *)? The alignment should normally
be good here, because we already are pointing to an address within
the header or block.
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
------------------------------
Message: 3
Date: Fri, 08 May 2015 18:07:13 +0200
From: Julien ?LIE <[email protected]>
To: "[email protected]" <[email protected]>
Subject: Re: Today's patches
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Matt,
>> #define TDX_MAGIC (~(0xf1f0f33d))
> Maybe somebody wanted something that looked like "fifo feed"?
Thanks to you and Bo for having pointed out the "f1f0 f33d" meaning!
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
------------------------------
Message: 4
Date: Fri, 8 May 2015 16:08:51 +0000
From: "Matt Seitz (matseitz)" <[email protected]>
To: Julien ?LIE <[email protected]>, "[email protected]"
<[email protected]>
Subject: RE: Today's patches
Message-ID:
<[email protected]>
Content-Type: text/plain; charset="utf-8"
Maybe it would be worth adding a comment, just to avoid future confusion?
-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Julien ?LIE
Sent: Friday, May 08, 2015 9:07
To: [email protected]
Subject: Re: Today's patches
Hi Matt,
>> #define TDX_MAGIC (~(0xf1f0f33d))
> Maybe somebody wanted something that looked like "fifo feed"?
Thanks to you and Bo for having pointed out the "f1f0 f33d" meaning!
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
Message: 5
Date: Fri, 08 May 2015 18:41:03 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Subject: Re: Today's patches
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
Hi Richard,
gcc warns about the new initialization { 0 }. Shouldn't the struct
be initialized with as many "0" as there are elements in the struct?
art.c:448:24: warning: missing field 'data' initializer
[-Wmissing-field-initializers]
ARTHANDLE arth = { 0 };
>>> Subject: [PATCH 2/3] Remove redundant (broken!) code
>>>
>>> The check was (i) off by one and (ii) can never happen, given the
>>> loop condition.
>>
>> At the end of the loop, we have:
>>
>> parent = &entry->next.recno;
>> current = *parent;
>
> Yes - but that is too late for the call to entry_splice(), which is
> where the user-after-munmap (or use-after-free) would occur. So parent
> must be recomputed somehow if a remap occurs. The conservative way to
> do it would be just to start again from the top.
Suggestion of patch:
--- tradindexed/tdx-group.c (r?vision 9852)
+++ tradindexed/tdx-group.c (copie de travail)
@@ -359,7 +359,7 @@
their next entry is entry 0. We don't want to leave things in this
state (particularly if this was the first expansion of the index file,
in which case entry 0 points to entry 0 and our walking functions may
- go into infinite loops. Undo the file expansion. */
+ go into infinite loops). Undo the file expansion. */
if (!index_map(index)) {
index->count -= 1024;
if (ftruncate(index->fd, index_file_size(index->count)) < 0) {
@@ -558,11 +558,20 @@
parent = &index->header->hash[index_bucket(hash)].recno;
current = *parent;
- while (current >= 0 && current < index->count) {
+ while (current >= 0) {
struct group_entry *entry;
- if (current > index->count && !index_maybe_remap(index, current))
- return -1;
+ if (current >= index->count) {
+ if (!index_maybe_remap(index, current)) {
+ return -1;
+ }
+ parent = &index->header->hash[index_bucket(hash)].recno;
+ current = *parent;
+ if (current < 0 || current >= index->count) {
+ syswarn("tradindexed: entry %ld out of range", current);
+ return -1;
+ }
+ }
entry = &index->entries[current];
if (entry->deleted == 0)
if (memcmp(&hash, &entry->hash, sizeof(hash)) == 0) {
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
------------------------------
Message: 6
Date: Fri, 08 May 2015 18:42:06 +0200
From: Julien ?LIE <[email protected]>
To: "[email protected]" <[email protected]>
Subject: Re: Today's patches
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Matt,
>>>> #define TDX_MAGIC (~(0xf1f0f33d))
>>> Maybe somebody wanted something that looked like "fifo feed"?
>> Thanks to you and Bo for having pointed out the "f1f0 f33d" meaning!
> Maybe it would be worth adding a comment, just to avoid future confusion?
Good idea; I'll add a comment.
--
Julien ?LIE
? ? Est-ce que tu peux remettre en ?tat un druide qui a d?rap?
sur une roche d'huile ?
? Euh ?!? C'est un accident peu fr?quent, mais je pense pouvoir y
parvenir ! ? (Ast?rix)
------------------------------
Message: 7
Date: Fri, 8 May 2015 16:57:42 +0000
From: "Matt Seitz (matseitz)" <[email protected]>
To: Julien ?LIE <[email protected]>, "[email protected]"
<[email protected]>
Subject: RE: Today's patches
Message-ID:
<[email protected]>
Content-Type: text/plain; charset="utf-8"
ISO Standard C says that if an initializer initializes some but not all
elements, the additional elements are automatically initialized as if "0" was
explicitly given. This makes it nice to use "={0}" as a default initializer,
without having to update the initializer every time the elements in a struct
are changed.
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
Section 6.7.9:
21. If there are fewer initializers in a brace-enclosed list than there are
elements or members
of an aggregate, or fewer characters in a string literal used to initialize an
array of known
size than there are elements in the array, the remainder of the aggregate shall
be
initialized implicitly the same as objects that have static storage duration.
Meaning, any items not explicitly initialized will be initialized according to
these rules:
10 If an object that has automatic storage duration is not initialized
explicitly, its value is
indeterminate. If an object that has static or thread storage duration is not
initialized
explicitly, then:
? if it has pointer type, it is initialized to a null pointer;
? if it has arithmetic type, it is initialized to (positive or unsigned) zero;
? if it is an aggregate, every member is initialized (recursively) according to
these rules,
and any padding is initialized to zero bits;
? if it is a union, the first named member is initialized (recursively)
according to these
rules, and any padding is initialized to zero bits;
-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Julien ?LIE
Sent: Friday, May 08, 2015 9:41
To: [email protected]
Subject: Re: Today's patches
Hi Richard,
gcc warns about the new initialization { 0 }. Shouldn't the struct be
initialized with as many "0" as there are elements in the struct?
art.c:448:24: warning: missing field 'data' initializer
[-Wmissing-field-initializers]
ARTHANDLE arth = { 0 };
>>> Subject: [PATCH 2/3] Remove redundant (broken!) code
>>>
>>> The check was (i) off by one and (ii) can never happen, given the
>>> loop condition.
>>
>> At the end of the loop, we have:
>>
>> parent = &entry->next.recno;
>> current = *parent;
>
> Yes - but that is too late for the call to entry_splice(), which is
> where the user-after-munmap (or use-after-free) would occur. So
> parent must be recomputed somehow if a remap occurs. The conservative
> way to do it would be just to start again from the top.
Suggestion of patch:
--- tradindexed/tdx-group.c (r?vision 9852)
+++ tradindexed/tdx-group.c (copie de travail)
@@ -359,7 +359,7 @@
their next entry is entry 0. We don't want to leave things in this
state (particularly if this was the first expansion of the index file,
in which case entry 0 points to entry 0 and our walking functions may
- go into infinite loops. Undo the file expansion. */
+ go into infinite loops). Undo the file expansion. */
if (!index_map(index)) {
index->count -= 1024;
if (ftruncate(index->fd, index_file_size(index->count)) < 0) { @@
-558,11 +558,20 @@
parent = &index->header->hash[index_bucket(hash)].recno;
current = *parent;
- while (current >= 0 && current < index->count) {
+ while (current >= 0) {
struct group_entry *entry;
- if (current > index->count && !index_maybe_remap(index, current))
- return -1;
+ if (current >= index->count) {
+ if (!index_maybe_remap(index, current)) {
+ return -1;
+ }
+ parent = &index->header->hash[index_bucket(hash)].recno;
+ current = *parent;
+ if (current < 0 || current >= index->count) {
+ syswarn("tradindexed: entry %ld out of range", current);
+ return -1;
+ }
+ }
entry = &index->entries[current];
if (entry->deleted == 0)
if (memcmp(&hash, &entry->hash, sizeof(hash)) == 0) {
--
Julien ?LIE
? Vinum bonum laetificat cor hominis. ?
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
End of inn-workers Digest, Vol 72, Issue 6
******************************************
