Tackling the offsetof warning with Non Standard Layout classes

Development OS
#1

I thought of starting my Haiku development journey by tackling on EasyTasks lists issue #9460. There are two types of violations related to Standard Layout as I can see.

   1 no virtual functions or virtual base classes
   2 all non-static data members have the same access control
   3 all non-static members of class type are standard-layout
   4 any base classes are standard-layout
   5 has no base classes of the same type as the first non-static data member.
   6 meets one of these conditions:
        * no non-static data member in the most-derived class and no more than one base class with non-static data members, or
        * has no base classes with non-static data members

First one is of access control, which I was able to fix by creating a Static member function for calculating the offset of a private member variable.
Second one is more hard, which doesn’t allow any virtual functions in the base classes. We can bypass the warning by just replacing the macro with its actual value of
#define XFS_EXTENT_CRC_OFF ((size_t)(&((ExtentDataHeaderV5*)0)->crc))
instead of
#define XFS_EXTENT_CRC_OFF offsetof(ExtentDataHeaderV5, crc)
But this would be clearly unsafe.
I tried researching online about how we can avoid the usage of virtual functions, one of the options was using templates, which I wasn’t able to wrap my mind around.
Another was solution(?) can be to remove virtual keyword, again making it unsafe to use.

So, what is the correct way to move forward with this? Any help is appreciated.

#2

Move the data members into a separate standard layout class (which would then be a single member of the class having virtual functions).

Another option (but maybe not practical here) could be the use of pointers-to-members.

#3

Yes, in the case of the XFS code I would probably go with this option. The reason for this is that the data is directly mapped from on-disk data and so these structures should match the on-disk data (same padding and offsets) so there is no place for virtual functions there. Unfortunately this was missed during the last round of work on XFS.

So we would have:

  • An abstract class with virtual methods
  • Implementations of this class for v4 and v5 XFS, each containing a simple structure with the data, and accessing that
  • The offsetof can now refer to the inner structure which wouldn’t have any methods

It will be a bit more complicated than what we have now, but it will make it clearer that this data is defined from on-disk formats of XFS and can’t be modified. And the classes can have extra fields if they need to.

#4

Are we talking about something like this?

class ExtentDataHeaderV4 : public ExtentDataHeader {
public :

								ExtentDataHeaderV4(const char* buffer);
								~ExtentDataHeaderV4();
			void				SwapEndian();
			uint32				Magic();
			uint64				Blockno();
			uint64				Lsn();
			uint64				Owner();
			uuid_t*				Uuid();
			static size_t		Offset_Data_magic()
								{ return offsetof(Data, magic); }
			

struct	Data{
	public:
			uint32				magic;
	private:
			FreeRegion			bestfree[XFS_DIR2_DATA_FD_COUNT];		
} Data_var; 

};

or a separate Data class for each version defined outside the current class?

Update:
So, I implemented the separate classes as below only for V4. Wanna get it confirmed that everything is OK, before implementing for V5.

struct	DataV4{
	public:
			static size_t		Offset_magic()
								{ return offsetof(DataV4, magic); }
			uint32				get_magic()
								{ return magic; }
			void				set_magic(uint32 newVal)
								{ magic = newVal; }
			FreeRegion*			get_bestfree()
								{ return bestfree; }
			
	private:
			uint32				magic;
			FreeRegion			bestfree[XFS_DIR2_DATA_FD_COUNT];		
};


//xfs_dir_data_hdr_t
class ExtentDataHeaderV4 : public ExtentDataHeader {
public :

								ExtentDataHeaderV4(const char* buffer);
								~ExtentDataHeaderV4();
			void				SwapEndian();
			uint32				Magic();
			uint64				Blockno();
			uint64				Lsn();
			uint64				Owner();
			uuid_t*				Uuid();
			
			DataV4				Data_var;

};
#5

What I would do is:

class ExtentDataHeaderV4 : public ExtentDataHeader {
public :

								ExtentDataHeaderV4(const char* buffer);
								~ExtentDataHeaderV4();
			void				SwapEndian();
			uint32				Magic() { return fOnDiskData.magic; }
			uint64				Blockno();
			uint64				Lsn();
			uint64				Owner();
			uuid_t*				Uuid();
			
private:
			struct	OnDiskData {
	        public:
			    uint32				magic;
			    // other on-disk data here	
            };

            OnDiskData fOnDiskData;
};

and a similar one for the v5 version, with a different OnDiskData structure matching the on-disk info for version 5 XFS

#6

Got it. Thanks for the help, and sorry for so many questions, just wanted to make sure I’m doing things properly.

2 Likes