Page 2 of 2
Re: EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 4:54 am
by mikegonta
dozniak wrote:mikegonta wrote:Here's something for you to
read. And, most importantly,
this part.
In that discussion, James-M is the only guy who really knows something about C compilers - he writes them for a living.
JamesM wrote:That's why I personally would mark every struct you need packed manually, as packed.
And I agree, but why mark a
struct packed that is already packed.
I should point out that
mikegonta wrote:Please
don't pack already "packed" structs.
is a redundant statement (and not
advice) much along the lines of "If it's not broken, don't fix it" and we all know how
misguided that one is.
dozniak wrote: So thanks for the read, I'm going to repeat now - "That's like the most misguided C advice I've ever heard on this forum from person who has
little to no idea about how C works."
It would probably be better to present your reasons why you disagree and engage in the discussion, rather than seek
advice from these forums.
The only thing worst than unsolicited advice is unqualified expert testimony.
Re: [Solved] EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 5:11 am
by mikegonta
Boris wrote:is there a way to tell your C compiler :
I don't care about how this struct looks in memory. You can reverse members order, skip those ever written to or read.
You can't if you want to interface (at a high level) with on device
structs; for example copying between the two. Using
unsigned char
array is portable with less chance that the compiler will do something unexpected at the cost of giving up the simplicity of
struct
assignment and notation and requiring the use of possibly error prone
#defines.
Boris wrote:Optimise using Lto and do it according to my arch.
Ultimately every
compile is arch specific, the main concern is how much the code has to be changed to adapt to a different arch.
LTO is promising, but not a panacea.
Re: EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 7:38 am
by matt11235
mikegonta wrote:And I agree, but why mark a struct packed that is already packed.
Just because it is packed on one platform doesn't mean that it is packed for every platform.
Re: EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 9:18 am
by mikegonta
matt11235 wrote:mikegonta wrote:And I agree, but why mark a struct packed that is already packed.
Just because it is packed on one platform doesn't mean that it is packed for every platform.
Agreed.
But on-device
structs packed or otherwise are not platform portable anyways.
So, if your arch specific platform requires packing, packing is available.
And off course it's best not to second guess the compiler, but then again it's probably best not to
guess when it comes to programming.
Re: [Solved] EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 10:28 am
by Kevin
Why would a filesystem driver be platform specific? This doesn't make any sense at all.
Also, can you please explain how using a char array instead of a proper struct fixes endianess issues? I could see how a char array instead of an int can be used to do this (in a rather tedious way compared to the usual conversion functions), but that's not related to structs at all.
Re: [Solved] EXT2 strangeness, again :(
Posted: Wed Jan 25, 2017 11:55 am
by mikegonta
Kevin wrote:Why would a filesystem driver be platform specific? This doesn't make any sense at all.
Of course it's not the file system driver that's platform specific but rather how the platform deals with the on-device
structs.
Kevin wrote:Also, can you please explain how using a char array instead of a proper struct fixes endianess issues? I could see how a char array
instead of an int can be used to do this (in a rather tedious way compared to the usual conversion functions), but that's not
related to structs at all.
Here's an example taken from
FATfs a generic cross platform FAT/exFAT file system driver used mainly for embedded devices.
Code: Select all
/* FatFs refers the members in the FAT structures as byte array instead of
/ structure members because the structure is not binary compatible between
/ different platforms */
#define BS_JmpBoot 0 /* x86 jump instruction (3-byte) */
#define BS_OEMName 3 /* OEM name (8-byte) */
#define BPB_BytsPerSec 11 /* Sector size [byte] (WORD) */
#define BPB_SecPerClus 13 /* Cluster size [sector] (BYTE) */
#define BPB_RsvdSecCnt 14 /* Size of reserved area [sector] (WORD) */
#define BPB_NumFATs 16 /* Number of FATs (BYTE) */
and here is one of several functions which access the
byte arrays in an endian neutral way.
Code: Select all
#if _FS_EXFAT
static
QWORD ld_qword (const BYTE* ptr) /* Load an 8-byte little-endian word */
{
QWORD rv;
rv = ptr[7];
rv = rv << 8 | ptr[6];
rv = rv << 8 | ptr[5];
rv = rv << 8 | ptr[4];
rv = rv << 8 | ptr[3];
rv = rv << 8 | ptr[2];
rv = rv << 8 | ptr[1];
rv = rv << 8 | ptr[0];
return rv;
}
#endif
And of course internally
structs are used and are the preferred "C" way.
Re: EXT2 strangeness, again :(
Posted: Thu Jan 26, 2017 4:38 pm
by mikegonta
LtG wrote:mikegonta wrote:Please
don't pack already "packed" structs.
Is it guaranteed that without "packed" the compiler will not add padding? Isn't it implementation specific? Granted most likely no
sane current compiler would add padding, but do you want to rely on that?
Even if it were guaranteed, and this is my main point, I'd mark it as packed to explicitly convey my intent. If the compiler is bad
and does something stupid then you should fix the compiler.
I guess the question is, why does GCC do a complex and unnecessary "memmove" instead of "rep movs"?
Actually, with
-march=i486,
memmove also uses
rep movsd. The extra code and complexity is for aligning the
loop. The
big problem
is that if you
pack already
packed structs
GCC can't/won't assume alignment.
LtG wrote:Can't you tell the compiler the alignment?
Apparently you can, (according to the linked article) which I can verify, by using
__attribute__((packed, aligned(x))) where
x is the
required byte alignment, in this case 4. With this there are no unwanted/unneeded
side effects.
Start
packing to your heart's content.
Case closed!
Re: EXT2 strangeness, again :(
Posted: Fri Jan 27, 2017 9:45 am
by dozniak
mikegonta wrote:Start packing to your heart's content.
Case closed!
Are you finally converted in the ways of GCC compiler then?
Re: EXT2 strangeness, again :(
Posted: Sun Jan 29, 2017 2:12 pm
by mikegonta
dozniak wrote:mikegonta wrote:Start packing to your heart's content.
Case closed!
Are you finally converted in the ways of GCC compiler then?
Absolutely not!
You still cannot substitute
__attribute__ ((__packed__)) for
__attribute__ ((__packed__, __aligned__())) in all cases.
If the
struct is already
packed all is good, the same as not using
__attribute__ ((__packed__)) in the first place.
However, if the
struct needs
packing __attribute__ ((__packed__, __aligned__())) will cause
GCC to transfer (in the case of
assignment) a number of bytes divisible by the alignment which could possibly cause overwriting of the on-device
struct.
GCC effectively puts the
padding at the end while preserving the internal structure.
Re: EXT2 strangeness, again :(
Posted: Sun Jan 29, 2017 2:49 pm
by dozniak
mikegonta wrote:
However, if the struct needs packing __attribute__ ((__packed__, __aligned__())) will cause GCC to transfer (in the case of
assignment) a number of bytes divisible by the alignment which could possibly cause overwriting of the on-device struct.
GCC effectively puts the padding at the end while preserving the internal structure.
Erm, this is documented behavior for aligned()
Doing this can often make copy operations more efficient, because the compiler can use whatever instructions copy the biggest chunks of memory when performing copies to or from the variables or fields that you have aligned this way.
When you specify
alignment gcc will use that as size-hint for copy operations, because structure will be aligned on these boundaries,
regardless of packed attribute.
If you want structure to be packed but also to be exactly as "on-device struct" (whatever that is and wherever you got it from) DO NOT USE ALIGNED!
Clear now?