colinux-devel

[coLinux-devel] offset calculation breakage

[Vladislav G. <the...@ma...>]

Dear Henry,

My Windows 7 x86 installation goes into BSOD right on/after console attach.
I've bisected it to recent r1560 changeset introduced co_offsetof macro.

> size = (char*)(&message->putcs + 1) - (char*)message + bytes_per_line;
Original construction means the struct length, excluding the member plus
size of one member, not its offset actually.

> #define co_offsetof(TYPE, MEMBER) ((int) &((TYPE *)0)->MEMBER)
> size = co_offsetof(co_console_message_t, putcs) + 1 + bytes_per_line; 
New one means the struct length excluding the member + one byte, that's
seems wrong.

So, to keep co_offsetof macro and restore binary compability, I suggest
following changes:
> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER))
> size = co_offsetof(co_console_message_t, putcs + 1) + bytes_per_line;

And, kindly please, is it possible to make daemons package to be built with
svn revision too.
Both patches attached, works for me.
Thank you.

Best Regards, theMIROn
ICQ: 303357
Skype: the.miron

Re: [coLinux-devel] offset calculation breakage

[Henry N. <hen...@ar...>]

Hello Vladislav,


On 10.01.2011 21:36, Vladislav Grishenko wrote:
> Dear Henry,
>
> My Windows 7 x86 installation goes into BSOD right on/after console attach.
> I've bisected it to recent r1560 changeset introduced co_offsetof macro.
>
>> size = (char*)(&message->putcs + 1) - (char*)message + bytes_per_line;
> Original construction means the struct length, excluding the member plus
> size of one member, not its offset actually.
>
>> #define co_offsetof(TYPE, MEMBER) ((int)&((TYPE *)0)->MEMBER)
>> size = co_offsetof(co_console_message_t, putcs) + 1 + bytes_per_line;
> New one means the struct length excluding the member + one byte, that's
> seems wrong.
>
> So, to keep co_offsetof macro and restore binary compability, I suggest
> following changes:
>> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER))
>> size = co_offsetof(co_console_message_t, putcs + 1) + bytes_per_line;

many thanks for pointing to that bug!

You are right, that this should give the size for struct without the 
data self.
The +1 was to point directly behind the element "putcs".
Your changes was not working on my test with gcc 4.4.1:

// Building:
//   DIR=$HOME/colinux/build/devel-gcc412.svn
//   gcc -m32 -I$DIR/linux-2.6.33.5-source/include 
-I$DIR/linux-2.6.33.5-build/include 
-I$DIR/linux-2.6.33.5-source/arch/x86/include foo.c

#include <stdio.h>
#include <linux/cooperative.h>

#define bytes_per_line 0
#define co_offsetof(TYPE, MEMBER) ((int) &((TYPE *)0)->MEMBER)

int main()
{
     co_console_message_t*<->message;
     int size1 = (char*)(&message->putcs + 1) - (char*)message + 
bytes_per_line;    // old
     int size2 = co_offsetof(co_console_message_t, putcs) + 1 + 
bytes_per_line;    // Henry
     int size3 = co_offsetof(co_console_message_t, putcs + 1) + 
bytes_per_line;    // Vladislav
     int size4 = co_offsetof(co_console_message_t, putcs.data) + 
bytes_per_line;    // right
     return printf("%d %d %d %d\n", size1, size2, size3, size4);
}

Result:
10 5 5 10

So, the right calculation would be:

-       size = co_offsetof(co_console_message_t, putcs) + 1 + 
bytes_per_line;
+       size = co_offsetof(co_console_message_t, putcs.data) + 
bytes_per_line;

-- 
Henry N.

Re: [coLinux-devel] offset calculation breakage

[Vladislav G. <the...@ma...>]

Dear Henry,
Sure it was't work the way you wrote
> #define co_offsetof(TYPE, MEMBER) ((int) &((TYPE *)0)->MEMBER)

But proposed define had member-whatever-it-contains bracket escaping
> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER))
So, with MEMBER == field+const it will be equal to start_of_field +
const*size_of_filed
Anyway, I'd like to see it fixed in svn :)
Btw, there's a lot of places with old style calculations, I mean without
co_offsetof usage,  even right several lines below the discussing line.

Best Regards, theMIROn
ICQ: 303357
Skype: the.miron


> -----Original Message-----
> From: Henry Nestler [mailto:hen...@ar...]
> Sent: Tuesday, January 11, 2011 3:41 AM
> To: Vladislav Grishenko
> Cc: col...@li...
> Subject: Re: offset calculation breakage
> 
> Hello Vladislav,
> 
> 
> On 10.01.2011 21:36, Vladislav Grishenko wrote:
> > Dear Henry,
> >
> > My Windows 7 x86 installation goes into BSOD right on/after console
> attach.
> > I've bisected it to recent r1560 changeset introduced co_offsetof macro.
> >
> >> size = (char*)(&message->putcs + 1) - (char*)message +
> >> bytes_per_line;
> > Original construction means the struct length, excluding the member
> > plus size of one member, not its offset actually.
> >
> >> #define co_offsetof(TYPE, MEMBER) ((int)&((TYPE *)0)->MEMBER) size =
> >> co_offsetof(co_console_message_t, putcs) + 1 + bytes_per_line;
> > New one means the struct length excluding the member + one byte,
> > that's seems wrong.
> >
> > So, to keep co_offsetof macro and restore binary compability, I
> > suggest following changes:
> >> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER)) size
> >> = co_offsetof(co_console_message_t, putcs + 1) + bytes_per_line;
> 
> many thanks for pointing to that bug!
> 
> You are right, that this should give the size for struct without the data
self.
> The +1 was to point directly behind the element "putcs".
> Your changes was not working on my test with gcc 4.4.1:
> 
> // Building:
> //   DIR=$HOME/colinux/build/devel-gcc412.svn
> //   gcc -m32 -I$DIR/linux-2.6.33.5-source/include
> -I$DIR/linux-2.6.33.5-build/include
> -I$DIR/linux-2.6.33.5-source/arch/x86/include foo.c
> 
> #include <stdio.h>
> #include <linux/cooperative.h>
> 
> #define bytes_per_line 0
> #define co_offsetof(TYPE, MEMBER) ((int) &((TYPE *)0)->MEMBER)
> 
> int main()
> {
>      co_console_message_t*<->message;
>      int size1 = (char*)(&message->putcs + 1) - (char*)message +
> bytes_per_line;    // old
>      int size2 = co_offsetof(co_console_message_t, putcs) + 1 +
> bytes_per_line;    // Henry
>      int size3 = co_offsetof(co_console_message_t, putcs + 1) +
> bytes_per_line;    // Vladislav
>      int size4 = co_offsetof(co_console_message_t, putcs.data) +
> bytes_per_line;    // right
>      return printf("%d %d %d %d\n", size1, size2, size3, size4); }
> 
> Result:
> 10 5 5 10
> 
> So, the right calculation would be:
> 
> -       size = co_offsetof(co_console_message_t, putcs) + 1 +
> bytes_per_line;
> +       size = co_offsetof(co_console_message_t, putcs.data) +
> bytes_per_line;
> 
> --
> Henry N.

Re: [coLinux-devel] offset calculation breakage

[Henry N. <hen...@ar...>]

Hello Vladislav,

On 11.01.2011 00:13, Vladislav Grishenko wrote:
> Dear Henry,
> Sure it was't work the way you wrote
>> #define co_offsetof(TYPE, MEMBER) ((int)&((TYPE *)0)->MEMBER)
> But proposed define had member-whatever-it-contains bracket escaping
>> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER))
> So, with MEMBER == field+const it will be equal to start_of_field +
> const*size_of_filed

OK. That give the test result:
10 5 10 10

Is the "MEMBER + const" right in usage of that macro?
Or is that a side effect in calculation?

> Anyway, I'd like to see it fixed in svn :)
> Btw, there's a lot of places with old style calculations, I mean without
> co_offsetof usage,  even right several lines below the discussing line.

I only changed that line in revision 1560, because the GCC 4.4.1 under 
Linux as host has a warning.
If you have more such changes, be welcome.

-- 
Henry N.

Re: [coLinux-devel] offset calculation breakage

[Vladislav G. <the...@ma...>]

Dear Henry,

> OK. That give the test result:
> 10 5 10 10
> 
> Is the "MEMBER + const" right in usage of that macro?
> Or is that a side effect in calculation?

It's right only within braces but the logic isn't good due non clear struct
pointer arithmetics.
Your committed variant seems to be more readable and fixes my issue, of
course.

Thank you and
Best Regards, theMIROn
ICQ: 303357
Skype: the.miron

> -----Original Message-----
> From: Henry Nestler [mailto:hen...@ar...]
> Sent: Tuesday, January 11, 2011 5:01 AM
> To: Vladislav Grishenko
> Cc: col...@li...
> Subject: Re: [coLinux-devel] offset calculation breakage
> 
> Hello Vladislav,
> 
> On 11.01.2011 00:13, Vladislav Grishenko wrote:
> > Dear Henry,
> > Sure it was't work the way you wrote
> >> #define co_offsetof(TYPE, MEMBER) ((int)&((TYPE *)0)->MEMBER)
> > But proposed define had member-whatever-it-contains bracket escaping
> >> #define co_offsetof(TYPE, MEMBER) ((int) (&((TYPE *)0)->MEMBER))
> > So, with MEMBER == field+const it will be equal to start_of_field +
> > const*size_of_filed
> 
> OK. That give the test result:
> 10 5 10 10
> 
> Is the "MEMBER + const" right in usage of that macro?
> Or is that a side effect in calculation?
> 
> > Anyway, I'd like to see it fixed in svn :) Btw, there's a lot of
> > places with old style calculations, I mean without co_offsetof usage,
> > even right several lines below the discussing line.
> 
> I only changed that line in revision 1560, because the GCC 4.4.1 under
Linux
> as host has a warning.
> If you have more such changes, be welcome.
> 
> --
> Henry N.
> 
> 
>
----------------------------------------------------------------------------
--
> Gaining the trust of online customers is vital for the success of any
company
> that requires sensitive data to be transmitted over the Web.   Learn how
to
> best implement a security strategy that keeps consumers' information
> secure
> and instills the confidence they need to proceed with transactions.
> http://p.sf.net/sfu/oracle-sfdevnl
> _______________________________________________
> coLinux-devel mailing list
> coL...@li...
> https://lists.sourceforge.net/lists/listinfo/colinux-devel