OK, here is another one. In the top level read_events() function in
fs/aio.c, a struct io_event is instantiated on the stack (variable ent).
It calls aio_read_evt() function which will fill the entire io_event
structure into variable ent. What's the point of zeroing when copy
covers the same memory area? Possible a debug code left around?
- Ken
<<aio.ent.patch>>
> OK, here is another one. In the top level read_events() function in
> fs/aio.c, a struct io_event is instantiated on the stack (variable ent).
> It calls aio_read_evt() function which will fill the entire io_event
> structure into variable ent. What's the point of zeroing when copy
> covers the same memory area? Possible a debug code left around?
Read the comment before that memset. The structure might contain some
padding (bytes not belonging to any of its entries), these bytes are
random and if you do not zero them, you copy random data into userspace.
Mikulas
> > OK, here is another one. In the top level read_events() function in
> > fs/aio.c, a struct io_event is instantiated on the stack
> (variable ent).
> > It calls aio_read_evt() function which will fill the entire io_event
> > structure into variable ent. What's the point of zeroing when copy
> > covers the same memory area? Possible a debug code left around?
>
> Read the comment before that memset. The structure might contain some
> padding (bytes not belonging to any of its entries), these bytes are
> random and if you do not zero them, you copy random data into
> userspace.
That is true, but here's the definition of the io_event strcuture:
struct io_event {
__u64 data;
__u64 obj;
__s64 res;
__s64 res2;
};
In the words of the comment, C may be "fun", but I've
having trouble envisioning an architecture where a structure
that consists of four equal sized objects has some padding!
Don't we usually call code that defends against impossible
problems "bloat"?
-Tony
Luck, Tony wrote:
> That is true, but here's the definition of the io_event strcuture:
>
> struct io_event {
> __u64 data;
> __u64 obj;
> __s64 res;
> __s64 res2;
> };
>
> In the words of the comment, C may be "fun", but I've
> having trouble envisioning an architecture where a structure
> that consists of four equal sized objects has some padding!
<newbie>
There might be some architecture that requires 16 byte alignment...
how about surrounding the memcpy with if (sizeof(struct io_event) != 4 * sizeof(__u64)) ?
</newbie>
--
Dan Kegel
http://www.kegel.com
http://counter.li.org/cgi-bin/runscript/display-person.cgi?user=78045
> <newbie>
> There might be some architecture that requires 16 byte alignment...
> how about surrounding the memcpy with
> if (sizeof(struct io_event) != 4 * sizeof(__u64)) ?
> </newbie>
The point I'm trying to make is that it is irrelevant with respect to
alignment, size, or padding. memset and struct copying has the same
length and destination address. Why bother with the memset? It's the
same as writing a code like this:
blah = 0;
blah = foo;
- Ken
>> OK, here is another one. In the top level read_events() function in
>> fs/aio.c, a struct io_event is instantiated on the stack (variable
ent).
>> It calls aio_read_evt() function which will fill the entire io_event
>> structure into variable ent. What's the point of zeroing when copy
>> covers the same memory area? Possible a debug code left around?
>
>Read the comment before that memset. The structure might contain some
>padding (bytes not belonging to any of its entries), these bytes are
>random and if you do not zero them, you copy random data into
userspace.
>
>Mikulas
Please have the courtesy of reading the relevant aio code in the entire
context. The comment doesn't make any sense for the current state of
the code. My guess is that it is left from stone age.
read_events(...) {
struct io_event ent;
memset(&ent, 0, sizeof(ent));
while (...) {
aio_read_evt(ctx, &ent);
}
...
}
read_events calls aio_read_evt and passes pointer to ent as argument.
aio_read_evt(..., struct io_event *ent) {
...
*ent = *evp;
...
}
Where on earth is the padding with respect to memset and struct copying?
If you are still not convinced and having doubts in understanding the C
code, then look at the assembly:
In read_events():
e64: 31 c0 xor %eax,%eax
....
e98: b9 08 00 00 00 mov $0x8,%ecx
e9d: f3 ab repz stos %eax,%es:(%edi)
<- 32 byte memset ->
In aio_read_evt()
ddf: 8b 34 24 mov (%esp,1),%esi
de2: b9 08 00 00 00 mov $0x8,%ecx
de7: f3 a5 repz movsl %ds:(%esi),%es:(%edi)
<- 32 byte structure copying ->
Now tell me again where on earth it has anything to do with padding? If
there is any padding going on, it is overridden by the source of the
structure copying, effectively nullify the attempt of memset.
- Ken