linux-kernel - Re: [PATCH] fs: Add new pre-allocation ioctls to vfs for compatibility with legacy xfs ioctls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 01 Feb 2009 14:32:45 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
CC:	Arnd Bergmann <arnd@...db.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ankit Jain <me@...itjain.org>, viro@...iv.linux.org.uk,
	hch@...radead.org, linux-fsdevel@...r.kernel.org, mfasheh@...e.com,
	joel.becker@...cle.com, ocfs2-devel@....oracle.com,
	linux-kernel@...r.kernel.org, xfs-masters@....sgi.com,
	xfs@....sgi.com
Subject: Re: [PATCH] fs: Add new pre-allocation ioctls to vfs for compatibility
 with legacy xfs ioctls

Geert Uytterhoeven wrote:
> On Sun, 1 Feb 2009, Boaz Harrosh wrote:
>> Geert Uytterhoeven wrote:
>>> On Sun, 1 Feb 2009, Boaz Harrosh wrote:
>>>> Arnd Bergmann wrote:
>>>>> +struct space_resv {
>>>>> +	__s16		l_type;
>>>>> +	__s16		l_whence;
>>>>> +	__s64		l_start;
>>>>> +	__s64		l_len;		/* len == 0 means until end of file */
>>>>> +	__s32		l_sysid;
>>>>> +	__u32		l_pid;
>>>>> +	__s32		l_pad[4];	/* reserve area			    */
>>>>> +};
>>>> What about telling the compiler exactly what you said above, just
>>>> to be sure we all mean the same thing. (And as documentation for new
>>>> comers):
>>>>
>>>> +struct space_resv_64 {
>>>> +	__s16		l_type;
>>>> +	__s16		l_whence;
>>>> +	__u32		reserved;
>>>> +	__s64		l_start;
>>>> +	__s64		l_len;		/* len == 0 means until end of file */
>>>> +	__s32		l_sysid;
>>>> +	__u32		l_pid;
>>>> +	__s32		l_pad[4];	/* reserve area			    */
>>>> +} __packed;
>>> Because the compiler will assume all fields are always unaligned and will use very
>>> suboptimal code to access them?
>> This discussion comes up every once in a while. I'm using an old FC7 compiler
>> (gcc (GCC) 4.1.2 20070925 (Red Hat 4.1.2-27)) And tests show that when the layout
>> of a structure is exactly the same the "__packed" on structure declarations does
>> nothing. It only starts to affect when there are real differences in alignment.
>> Also tests with gcc 3.4.x showed the same effect.
>>
>> On previous discussions no one could come forward and say what compiler version
>> breaks when __packed is applied on structure definition. I'm afraid your statement
>> above is a myth.
> 
> FC7, targeting ia32? Sure, ia32 has no alignment restrictions.
> Try e.g. MIPS.
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 

I don't understand

if you have a structure like
struct foo {
	u32 one;
	u32 two;
};
vs
struct foo_packed {
	u32 one;
	u32 two;
} __packed;

Just adding an __attribute__((packed)) to it clearly does not change
the layout of the structure. Are you saying the __attribute__((packed))
is an hint to the compiler that foo_packed might be used unaligned. This
is just brain-dead, because I can use an unaligned pointer to foo just as
I can to foo_packed. Otherwise there is no difference what-so-ever between
the two. I have to see it to believe. It is totally the wrong hint in the
wrong place taking away valuable meaning of saying "please don't use padding
holes in this structure"

Sorry for been so slow, I just don't get it.
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/