Skip to content

Commit 0849c91

Browse files
pulkomandypulkomandy
committed
xfs: Enable -Werror, fix all warnings
Define a dedicated OnDiskData structure for each on-disk structure. This must match the on-disk layout, except for endianness, which is handled by _SwapEndian methods. These structure are "plain old data" so we can use offsetof on them. They are wrapped in an easier to use C++ API. This resolves a lot of problems with the previous code: warnings caused by the use of offsetof as well as a much simpler instanciation of the objects from on-disk data. Also fixed another problem with UUIDs, where the UUIDs were handled by pointers in a lot of place where it was not necessary. Use references instead. The V4 structures which don't have an UUID will return a "null" (zero-filled) one. Change-Id: Ifb2bf6ab94906ca50410dd3446d3566615392ca2 Reviewed-on: https://review.haiku-os.org/c/haiku/+/6021 Reviewed-by: Raghav Sharma <[email protected]> Reviewed-by: Adrien Destugues <[email protected]> Tested-by: Commit checker robot <[email protected]>
1 parent 66196a8 commit 0849c91

17 files changed

+272
-350
lines changed

‎build/jam/ArchitectureRules

+1-1
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ rule ArchitectureSetupWarnings architecture
738738
EnableWerror src add-ons kernel file_systems reiserfs ;
739739
EnableWerror src add-ons kernel file_systems udf ;
740740
EnableWerror src add-ons kernel file_systems userlandfs ;
741-
# EnableWerror src add-ons kernel file_systems xfs ;
741+
EnableWerror src add-ons kernel file_systems xfs ;
742742
EnableWerror src add-ons kernel generic ;
743743
EnableWerror src add-ons kernel network ;
744744
EnableWerror src add-ons kernel partitioning_systems ;

‎src/add-ons/kernel/file_systems/xfs/BPlusTree.h

+5-3
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,15 @@ class LongBlock {
4242
uint64 Lsn()
4343
{ return B_BENDIAN_TO_HOST_INT64(bb_lsn); }
4444

45-
uuid_t* Uuid()
46-
{ return &bb_uuid; }
45+
const uuid_t& Uuid()
46+
{ return bb_uuid; }
4747

4848
uint64 Owner()
4949
{ return B_BENDIAN_TO_HOST_INT64(bb_owner); }
5050

51+
static uint32 Offset_v5()
52+
{ return offsetof(LongBlock, bb_blkno); }
53+
5154
static uint32 ExpectedMagic(int8 WhichDirectory,
5255
Inode* inode);
5356

@@ -62,7 +65,6 @@ class LongBlock {
6265
uint64 bb_rightsib;
6366

6467
// Version 5 fields start here
65-
public:
6668
uint64 bb_blkno;
6769
uint64 bb_lsn;
6870
uuid_t bb_uuid;

‎src/add-ons/kernel/file_systems/xfs/Extent.cpp

+30-59
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Extent::FillMapEntry(void* pointerToMap)
2828
{
2929
uint64 firstHalf = *((uint64*)pointerToMap);
3030
uint64 secondHalf = *((uint64*)pointerToMap + 1);
31-
//dividing the 128 bits into 2 parts.
31+
// dividing the 128 bits into 2 parts.
3232
firstHalf = B_BENDIAN_TO_HOST_INT64(firstHalf);
3333
secondHalf = B_BENDIAN_TO_HOST_INT64(secondHalf);
3434
fMap->br_state = (firstHalf >> 63);
@@ -76,9 +76,9 @@ Extent::Init()
7676
void* pointerToMap = DIR_DFORK_PTR(fInode->Buffer(), fInode->CoreInodeSize());
7777
FillMapEntry(pointerToMap);
7878
ASSERT(fMap->br_blockcount == 1);
79-
//TODO: This is always true for block directories
80-
//If we use this implementation for leaf directories, this is not
81-
//always true
79+
// TODO: This is always true for block directories
80+
// If we use this implementation for leaf directories, this is not
81+
// always true
8282
status_t status = FillBlockBuffer();
8383
if (status != B_OK)
8484
return status;
@@ -274,7 +274,7 @@ ExtentDataHeader::ExpectedMagic(int8 WhichDirectory, Inode* inode)
274274
uint32
275275
ExtentDataHeader::CRCOffset()
276276
{
277-
return XFS_EXTENT_CRC_OFF - XFS_EXTENT_V5_VPTR_OFF;
277+
return offsetof(ExtentDataHeaderV5::OnDiskData, crc);
278278
}
279279

280280

@@ -301,29 +301,23 @@ uint32
301301
ExtentDataHeader::Size(Inode* inode)
302302
{
303303
if (inode->Version() == 1 || inode->Version() == 2)
304-
return sizeof(ExtentDataHeaderV4) - XFS_EXTENT_V4_VPTR_OFF;
304+
return sizeof(ExtentDataHeaderV4::OnDiskData);
305305
else
306-
return sizeof(ExtentDataHeaderV5) - XFS_EXTENT_V5_VPTR_OFF;
306+
return sizeof(ExtentDataHeaderV5::OnDiskData);
307307
}
308308

309309

310310
void
311-
ExtentDataHeaderV4::SwapEndian()
311+
ExtentDataHeaderV4::_SwapEndian()
312312
{
313-
magic = B_BENDIAN_TO_HOST_INT32(magic);
313+
fData.magic = (B_BENDIAN_TO_HOST_INT32(fData.magic));
314314
}
315315

316316

317317
ExtentDataHeaderV4::ExtentDataHeaderV4(const char* buffer)
318318
{
319-
uint32 offset = 0;
320-
321-
magic = *(uint32*)(buffer + offset);
322-
offset += sizeof(uint32);
323-
324-
memcpy(bestfree, buffer + offset, XFS_DIR2_DATA_FD_COUNT * sizeof(FreeRegion));
325-
326-
SwapEndian();
319+
memcpy(&fData, buffer, sizeof(fData));
320+
_SwapEndian();
327321
}
328322

329323

@@ -335,7 +329,7 @@ ExtentDataHeaderV4::~ExtentDataHeaderV4()
335329
uint32
336330
ExtentDataHeaderV4::Magic()
337331
{
338-
return magic;
332+
return fData.magic;
339333
}
340334

341335

@@ -360,52 +354,29 @@ ExtentDataHeaderV4::Owner()
360354
}
361355

362356

363-
uuid_t*
357+
const uuid_t&
364358
ExtentDataHeaderV4::Uuid()
365359
{
366-
return NULL;
360+
static uuid_t nullUuid;
361+
return nullUuid;
367362
}
368363

369364

370365
void
371-
ExtentDataHeaderV5::SwapEndian()
366+
ExtentDataHeaderV5::_SwapEndian()
372367
{
373-
magic = B_BENDIAN_TO_HOST_INT32(magic);
374-
blkno = B_BENDIAN_TO_HOST_INT64(blkno);
375-
lsn = B_BENDIAN_TO_HOST_INT64(lsn);
376-
owner = B_BENDIAN_TO_HOST_INT64(owner);
377-
pad = B_BENDIAN_TO_HOST_INT32(pad);
368+
fData.magic = B_BENDIAN_TO_HOST_INT32(fData.magic);
369+
fData.blkno = B_BENDIAN_TO_HOST_INT64(fData.blkno);
370+
fData.lsn = B_BENDIAN_TO_HOST_INT64(fData.lsn);
371+
fData.owner = B_BENDIAN_TO_HOST_INT64(fData.owner);
372+
fData.pad = B_BENDIAN_TO_HOST_INT32(fData.pad);
378373
}
379374

380375

381376
ExtentDataHeaderV5::ExtentDataHeaderV5(const char* buffer)
382377
{
383-
uint32 offset = 0;
384-
385-
magic = *(uint32*)(buffer + offset);
386-
offset += sizeof(uint32);
387-
388-
crc = *(uint32*)(buffer + offset);
389-
offset += sizeof(uint32);
390-
391-
blkno = *(uint64*)(buffer + offset);
392-
offset += sizeof(uint64);
393-
394-
lsn = *(uint64*)(buffer + offset);
395-
offset += sizeof(uint64);
396-
397-
memcpy(&uuid, buffer + offset, sizeof(uuid_t));
398-
offset += sizeof(uuid_t);
399-
400-
owner = *(uint64*)(buffer + offset);
401-
offset += sizeof(uint64);
402-
403-
memcpy(bestfree, buffer + offset, XFS_DIR2_DATA_FD_COUNT * sizeof(FreeRegion));
404-
offset += XFS_DIR2_DATA_FD_COUNT * sizeof(FreeRegion);
405-
406-
pad = *(uint32*)(buffer + offset);
407-
408-
SwapEndian();
378+
memcpy(&fData, buffer, sizeof(fData));
379+
_SwapEndian();
409380
}
410381

411382

@@ -417,33 +388,33 @@ ExtentDataHeaderV5::~ExtentDataHeaderV5()
417388
uint32
418389
ExtentDataHeaderV5::Magic()
419390
{
420-
return magic;
391+
return fData.magic;
421392
}
422393

423394

424395
uint64
425396
ExtentDataHeaderV5::Blockno()
426397
{
427-
return blkno;
398+
return fData.blkno;
428399
}
429400

430401

431402
uint64
432403
ExtentDataHeaderV5::Lsn()
433404
{
434-
return lsn;
405+
return fData.lsn;
435406
}
436407

437408

438409
uint64
439410
ExtentDataHeaderV5::Owner()
440411
{
441-
return owner;
412+
return fData.owner;
442413
}
443414

444415

445-
uuid_t*
416+
const uuid_t&
446417
ExtentDataHeaderV5::Uuid()
447418
{
448-
return &uuid;
449-
}
419+
return fData.uuid;
420+
}

‎src/add-ons/kernel/file_systems/xfs/Extent.h

+42-30
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,16 @@ struct FreeRegion {
5151

5252

5353
// This class will act as interface for V4 and V5 data header
54-
class ExtentDataHeader {
54+
class ExtentDataHeader
55+
{
5556
public:
56-
5757
virtual ~ExtentDataHeader() = 0;
58-
virtual uint32 Magic() = 0;
59-
virtual uint64 Blockno() = 0;
60-
virtual uint64 Lsn() = 0;
61-
virtual uint64 Owner() = 0;
62-
virtual uuid_t* Uuid() = 0;
58+
virtual uint32 Magic() = 0;
59+
virtual uint64 Blockno() = 0;
60+
virtual uint64 Lsn() = 0;
61+
virtual uint64 Owner() = 0;
62+
virtual const uuid_t& Uuid() = 0;
63+
6364
static uint32 ExpectedMagic(int8 WhichDirectory,
6465
Inode* inode);
6566
static uint32 CRCOffset();
@@ -68,51 +69,62 @@ class ExtentDataHeader {
6869
};
6970

7071

71-
//xfs_dir_data_hdr_t
72-
class ExtentDataHeaderV4 : public ExtentDataHeader {
72+
// xfs_dir_data_hdr_t
73+
class ExtentDataHeaderV4 : public ExtentDataHeader
74+
{
7375
public :
76+
struct OnDiskData {
77+
public:
78+
uint32 magic;
79+
FreeRegion bestfree[XFS_DIR2_DATA_FD_COUNT];
80+
};
7481

7582
ExtentDataHeaderV4(const char* buffer);
7683
~ExtentDataHeaderV4();
77-
void SwapEndian();
7884
uint32 Magic();
7985
uint64 Blockno();
8086
uint64 Lsn();
8187
uint64 Owner();
82-
uuid_t* Uuid();
88+
const uuid_t& Uuid();
8389

84-
uint32 magic;
8590
private:
86-
FreeRegion bestfree[XFS_DIR2_DATA_FD_COUNT];
91+
void _SwapEndian();
92+
93+
private:
94+
OnDiskData fData;
8795
};
8896

8997

9098
// xfs_dir3_data_hdr_t
91-
class ExtentDataHeaderV5 : public ExtentDataHeader {
99+
class ExtentDataHeaderV5 : public ExtentDataHeader
100+
{
92101
public:
102+
struct OnDiskData {
103+
public:
104+
uint32 magic;
105+
uint32 crc;
106+
uint64 blkno;
107+
uint64 lsn;
108+
uuid_t uuid;
109+
uint64 owner;
110+
FreeRegion bestfree[XFS_DIR2_DATA_FD_COUNT];
111+
uint32 pad;
112+
};
113+
93114
ExtentDataHeaderV5(const char* buffer);
94115
~ExtentDataHeaderV5();
95-
void SwapEndian();
96116
uint32 Magic();
97117
uint64 Blockno();
98118
uint64 Lsn();
99119
uint64 Owner();
100-
uuid_t* Uuid();
101-
public:
102-
uint32 magic;
103-
uint32 crc;
120+
const uuid_t& Uuid();
121+
104122
private:
105-
uint64 blkno;
106-
uint64 lsn;
107-
uuid_t uuid;
108-
uint64 owner;
109-
FreeRegion bestfree[XFS_DIR2_DATA_FD_COUNT];
110-
uint32 pad;
111-
};
123+
void _SwapEndian();
112124

113-
#define XFS_EXTENT_CRC_OFF offsetof(ExtentDataHeaderV5, crc)
114-
#define XFS_EXTENT_V5_VPTR_OFF offsetof(ExtentDataHeaderV5, magic)
115-
#define XFS_EXTENT_V4_VPTR_OFF offsetof(ExtentDataHeaderV4, magic)
125+
private:
126+
OnDiskData fData;
127+
};
116128

117129

118130
// xfs_dir2_data_entry_t
@@ -131,7 +143,7 @@ struct ExtentUnusedEntry {
131143
uint16 freetag;
132144
// takes the value 0xffff
133145
uint16 length;
134-
// freetag+length overrides the inumber of an entry
146+
// freetag + length overrides the inumber of an entry
135147
uint16 tag;
136148
};
137149

‎src/add-ons/kernel/file_systems/xfs/Inode.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ Inode::VerifyInode() const
294294
return false;
295295
}
296296

297-
if(!fVolume->UuidEquals(&fNode->di_uuid)) {
297+
if(!fVolume->UuidEquals(fNode->di_uuid)) {
298298
ERROR("UUID is incorrect");
299299
return false;
300300
}
@@ -443,7 +443,7 @@ Inode::SizeOfLongBlock()
443443
if (Version() == 3)
444444
return sizeof(LongBlock);
445445
else
446-
return offsetof(struct LongBlock, bb_blkno);
446+
return LongBlock::Offset_v5();
447447
}
448448

449449

@@ -878,4 +878,4 @@ hashfunction(const char* name, int length)
878878
}
879879

880880
return hashVal;
881-
}
881+
}

0 commit comments

Comments
 (0)