Re: [PATCH 1/3] arch/x86: Use offsetof
* H. Peter Anvin <[EMAIL PROTECTED]> wrote:
>>> The right way to do it is:
>>>
>>> memset(&info.vm86plus, 0, sizeof info.vm86plus);
>>
>> If it's just one field _and_ we don't have padding we want to zero out -
>> certainly...
>
> It is - [comments removed for clarity]:
>
> struct kernel_vm86_struct {
> struct kernel_vm86_regs regs;
> #define VM86_TSS_ESP0 flags
> unsigned long flags;
> unsigned long screen_bitmap;
> unsigned long cpu_type;
> struct revectored_struct int_revectored;
> struct revectored_struct int21_revectored;
> struct vm86plus_info_struct vm86plus;
> struct pt_regs *regs32;
> };
hm, i'm wondering why it was done in such a complex way. Clearing a
struct field is always done via sizeof. Maybe we lost some alignment
assumption somewhere along the line?
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
Jan Engelhardt wrote: On Dec 26 2007 17:01, H. Peter Anvin wrote: @@ -215,7 +215,9 @@ asmlinkage int sys_vm86old(struct pt_reg ret = -EFAULT; if (tmp) goto out; - memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus); + memset(&info.vm86plus, 0, + offsetof(struct kernel_vm86_struct, regs32) - + offsetof(struct kernel_vm86_struct, vm86plus)); I do not think this makes it more readable... (int) -> (char *) would make it portable and readable, AFAICT. Pavel The right way to do it is: memset(&info.vm86plus, 0, sizeof info.vm86plus); Either way, downcasting a pointer to (int) is dangerous, even if this one occurrence happens to be in 32-bit-only code. Actually, it would be safe (although stupid) in this case since the difference would still be 32 bits or less. Doesn't make it any less wrong. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
On Dec 26 2007 17:01, H. Peter Anvin wrote: >> > @@ -215,7 +215,9 @@ asmlinkage int sys_vm86old(struct pt_reg >> > ret = -EFAULT; >> > if (tmp) >> >goto out; >> > - memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus); >> > + memset(&info.vm86plus, 0, >> > + offsetof(struct kernel_vm86_struct, regs32) - >> > + offsetof(struct kernel_vm86_struct, vm86plus)); >> >> I do not think this makes it more readable... (int) -> (char *) would >> make it portable and readable, AFAICT. >> Pavel > > The right way to do it is: > > memset(&info.vm86plus, 0, sizeof info.vm86plus); Either way, downcasting a pointer to (int) is dangerous, even if this one occurrence happens to be in 32-bit-only code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
Al Viro wrote:
On Wed, Dec 26, 2007 at 05:01:38PM -0800, H. Peter Anvin wrote:
The right way to do it is:
memset(&info.vm86plus, 0, sizeof info.vm86plus);
If it's just one field _and_ we don't have padding we want to zero out -
certainly...
It is - [comments removed for clarity]:
struct kernel_vm86_struct {
struct kernel_vm86_regs regs;
#define VM86_TSS_ESP0 flags
unsigned long flags;
unsigned long screen_bitmap;
unsigned long cpu_type;
struct revectored_struct int_revectored;
struct revectored_struct int21_revectored;
struct vm86plus_info_struct vm86plus;
struct pt_regs *regs32;
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
On Wed, Dec 26, 2007 at 05:01:38PM -0800, H. Peter Anvin wrote: > The right way to do it is: > > memset(&info.vm86plus, 0, sizeof info.vm86plus); If it's just one field _and_ we don't have padding we want to zero out - certainly... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
Pavel Machek wrote: diff -u -p a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c --- a/arch/x86/kernel/vm86_32.c 2007-10-22 11:25:00.0 +0200 +++ b/arch/x86/kernel/vm86_32.c 2007-12-26 16:27:15.0 +0100 @@ -215,7 +215,9 @@ asmlinkage int sys_vm86old(struct pt_reg ret = -EFAULT; if (tmp) goto out; - memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus); + memset(&info.vm86plus, 0, + offsetof(struct kernel_vm86_struct, regs32) - + offsetof(struct kernel_vm86_struct, vm86plus)); I do not think this makes it more readable... (int) -> (char *) would make it portable and readable, AFAICT. Pavel The right way to do it is: memset(&info.vm86plus, 0, sizeof info.vm86plus); -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] arch/x86: Use offsetof
Hi! > From: Julia Lawall <[EMAIL PROTECTED]> > > In the patch cc154ac64aa8d3396b187f64cef01ce67f433324, Al Viro observed > that the proper way to compute the distance between two structure fields is > to use offsetof() or a cast to a pointer to character. The same change can > be applied to a few more files. > > The change was made using the following semantic patch > (http://www.emn.fr/x-info/coccinelle/) > > // > @r@ > type T; > T s; > type T1, T2; > identifier fld1, fld2; > typedef uint8_t; > typedef u8; > @@ > > ( > (char *)&s.fld1 - (char *)&s.fld2 > | > (uint8_t *)&s.fld1 - (uint8_t *)&s.fld2 > | > (u8 *)&s.fld1 - (u8 *)&s.fld2 > | > - (T1)&s.fld1 - (T2)&s.fld2 > + offsetof(T,fld1) - offsetof(T,fld2) > ) > // > > Signed-off-by: Julia Lawall <[EMAIL PROTECTED]> > --- > > diff -u -p a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c > --- a/arch/x86/kernel/vm86_32.c 2007-10-22 11:25:00.0 +0200 > +++ b/arch/x86/kernel/vm86_32.c 2007-12-26 16:27:15.0 +0100 > @@ -215,7 +215,9 @@ asmlinkage int sys_vm86old(struct pt_reg > ret = -EFAULT; > if (tmp) > goto out; > - memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus); > + memset(&info.vm86plus, 0, > +offsetof(struct kernel_vm86_struct, regs32) - > +offsetof(struct kernel_vm86_struct, vm86plus)); I do not think this makes it more readable... (int) -> (char *) would make it portable and readable, AFAICT. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] arch/x86: Use offsetof
From: Julia Lawall <[EMAIL PROTECTED]> In the patch cc154ac64aa8d3396b187f64cef01ce67f433324, Al Viro observed that the proper way to compute the distance between two structure fields is to use offsetof() or a cast to a pointer to character. The same change can be applied to a few more files. The change was made using the following semantic patch (http://www.emn.fr/x-info/coccinelle/) // @r@ type T; T s; type T1, T2; identifier fld1, fld2; typedef uint8_t; typedef u8; @@ ( (char *)&s.fld1 - (char *)&s.fld2 | (uint8_t *)&s.fld1 - (uint8_t *)&s.fld2 | (u8 *)&s.fld1 - (u8 *)&s.fld2 | - (T1)&s.fld1 - (T2)&s.fld2 + offsetof(T,fld1) - offsetof(T,fld2) ) // Signed-off-by: Julia Lawall <[EMAIL PROTECTED]> --- diff -u -p a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c --- a/arch/x86/kernel/vm86_32.c 2007-10-22 11:25:00.0 +0200 +++ b/arch/x86/kernel/vm86_32.c 2007-12-26 16:27:15.0 +0100 @@ -215,7 +215,9 @@ asmlinkage int sys_vm86old(struct pt_reg ret = -EFAULT; if (tmp) goto out; - memset(&info.vm86plus, 0, (int)&info.regs32 - (int)&info.vm86plus); + memset(&info.vm86plus, 0, + offsetof(struct kernel_vm86_struct, regs32) - + offsetof(struct kernel_vm86_struct, vm86plus)); info.regs32 = ®s; tsk->thread.vm86_info = v86; do_sys_vm86(&info, tsk); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
