[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#898985: gdisk: FTBFS on sparc64, testsuite failure due to unaligned access



Control: tags -1 +patch

The attached patch fixes the problem. It was indeed an alignment issue
which was caused by the incorrect use of "#pragma pack ()" throughout
the code as discovered by James Clarke [1].

The problem is that changing the default alignment using "#pragma pack (n)"
to n bytes does not only apply to the struct following the #pragma directive
but the all of the remaining code followed by the statement which results
in all kinds of structs to become unaligned.

According to the gcc documentation, "#pragma pack ()" switches the alignment
back to the previous value which would be the native alignment for a given
architecture. I have just done this and gdisk now builds fine for me
on sparc64 and passes the testsuite.

I am also about to send a pull request upstream.

Thanks,
Adrian

> [1] https://sourceforge.net/p/gptfdisk/discussion/939590/thread/eaf714bb/?limit=25#92b4
> [2] https://gcc.gnu.org/onlinedocs/gcc-4.4.4/gcc/Structure_002dPacking-Pragmas.html

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>From c9dc1134413e8fa50f63354b78e84179b273896d Mon Sep 17 00:00:00 2001
From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: Fri, 18 May 2018 23:30:05 +0200
Subject: [PATCH] Reset struct alignment back to standard after each #pragma
 pack (n)

When using #pragma pack (n) to change the alignment for a struct
to n bytes, it is necessary to reset the alignment to the standard
value as otherwise the changed alignment is used throughout the
whole program code which causes other structs to become unaligned.
---
 basicmbr.h | 1 +
 bsd.h      | 1 +
 gpt.h      | 1 +
 mbrpart.h  | 1 +
 4 files changed, 4 insertions(+)

diff --git a/basicmbr.h b/basicmbr.h
index b809856..21f962e 100644
--- a/basicmbr.h
+++ b/basicmbr.h
@@ -34,6 +34,7 @@ struct TempMBR {
    struct MBRRecord partitions[4];
    uint16_t MBRSignature;
 }; // struct TempMBR
+#pragma pack ()
 
 // Possible states of the MBR
 enum MBRValidity {invalid, gpt, hybrid, mbr};
diff --git a/bsd.h b/bsd.h
index ffbe5cc..cbd3588 100644
--- a/bsd.h
+++ b/bsd.h
@@ -89,5 +89,6 @@ class BSDData {
       int GetNumParts(void);
       GPTPart AsGPT(int i); // Return BSD part. as GPT part.
 }; // struct MBRData
+#pragma pack ()
 
 #endif
diff --git a/gpt.h b/gpt.h
index 2d7a1ce..f4bf470 100644
--- a/gpt.h
+++ b/gpt.h
@@ -57,6 +57,7 @@ struct GPTHeader {
    uint32_t partitionEntriesCRC;
    unsigned char reserved2[GPT_RESERVED];
 }; // struct GPTHeader
+#pragma pack ()
 
 // Data in GPT format
 class GPTData {
diff --git a/mbrpart.h b/mbrpart.h
index f5892e7..0de365f 100644
--- a/mbrpart.h
+++ b/mbrpart.h
@@ -50,6 +50,7 @@ struct MBRRecord {
    uint32_t firstLBA; // see above
    uint32_t lengthLBA;
 }; // struct MBRRecord
+#pragma pack ()
 
 class MBRPart {
 protected:
-- 
2.17.0


Reply to: