Avoid using unions for LongDirEntry.

GCC still assumes that the dirent has no data past the end for some
scenarios here and still mis-optimizes things. Therefore, drop the
usages of unions altogether, and instead use a casted character array.

Additionally, use B_FILE_NAME_LENGTH for the array, not B_PATH_NAME_LENGTH,
and make sure to add 1 for the NULL terminator.
This commit is contained in:
Augustin Cavalier
2021-11-23 16:36:18 -05:00
parent 8be37ed439
commit 2532a28785
12 changed files with 38 additions and 39 deletions
+3 -3
View File
@@ -25,9 +25,9 @@ namespace BPrivate {
namespace Storage { namespace Storage {
// For convenience: // For convenience:
union LongDirEntry { struct LongDirEntry {
struct dirent dirent; char _[sizeof(struct dirent) + B_FILE_NAME_LENGTH + 1];
char _[sizeof(struct dirent) + B_PATH_NAME_LENGTH]; struct dirent* dirent() { return (struct dirent*)_; }
}; };
//! Returns whether the supplied path is absolute. //! Returns whether the supplied path is absolute.
+2 -2
View File
@@ -333,7 +333,7 @@ BDirectory::GetNextRef(entry_ref* ref)
return B_FILE_ERROR; return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
bool next = true; bool next = true;
while (next) { while (next) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
@@ -377,7 +377,7 @@ BDirectory::CountEntries()
return error; return error;
int32 count = 0; int32 count = 0;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
while (error == B_OK) { while (error == B_OK) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
break; break;
+1 -1
View File
@@ -110,7 +110,7 @@ status_t
BMergedDirectory::GetNextRef(entry_ref* ref) BMergedDirectory::GetNextRef(entry_ref* ref)
{ {
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* dirEntry = &longEntry.dirent; struct dirent* dirEntry = longEntry.dirent();
int32 result = GetNextDirents(dirEntry, sizeof(longEntry), 1); int32 result = GetNextDirents(dirEntry, sizeof(longEntry), 1);
if (result < 0) if (result < 0)
return result; return result;
+1 -1
View File
@@ -310,7 +310,7 @@ BNode::GetNextAttrName(char* buffer)
return B_FILE_ERROR; return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1); ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1);
if (result < 0) if (result < 0)
return result; return result;
+8 -8
View File
@@ -129,10 +129,10 @@ class AttributeDirectory;
typedef map<DIR*, AttributeDirectory*> AttrDirMap; typedef map<DIR*, AttributeDirectory*> AttrDirMap;
static AttrDirMap sAttributeDirectories; static AttrDirMap sAttributeDirectories;
// LongDirent // LongDirEntry
union LongDirent { struct LongDirEntry {
struct dirent dirent; char _[sizeof(struct dirent) + B_FILE_NAME_LENGTH + 1];
char _[sizeof(struct dirent) + B_FILE_NAME_LENGTH]; struct dirent* dirent() { return (struct dirent*)_; }
}; };
// AttributeHeader // AttributeHeader
@@ -260,11 +260,11 @@ public:
} }
// prepare the dirent // prepare the dirent
strcpy(fDirent.dirent.d_name, name); strcpy(fDirent.dirent()->d_name, name);
fDirent.dirent.d_ino = 0; fDirent.dirent()->d_ino = 0;
// TODO: We need the node ID! // TODO: We need the node ID!
*_entry = &fDirent.dirent; *_entry = fDirent.dirent();
return B_OK; return B_OK;
} }
@@ -309,7 +309,7 @@ private:
int fFileFD; int fFileFD;
string fPath; string fPath;
DIR* fFakeDir; DIR* fFakeDir;
LongDirent fDirent; LongDirEntry fDirent;
char* fListing; char* fListing;
int fListingLength; int fListingLength;
int fListingIndex; int fListingIndex;
+2 -2
View File
@@ -353,7 +353,7 @@ BDirectory::GetNextRef(entry_ref* ref)
return B_FILE_ERROR; return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
bool next = true; bool next = true;
while (next) { while (next) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
@@ -397,7 +397,7 @@ BDirectory::CountEntries()
return error; return error;
int32 count = 0; int32 count = 0;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
while (error == B_OK) { while (error == B_OK) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) if (GetNextDirents(entry, sizeof(longEntry), 1) != 1)
break; break;
+1 -1
View File
@@ -112,7 +112,7 @@ status_t
BMergedDirectory::GetNextRef(entry_ref* ref) BMergedDirectory::GetNextRef(entry_ref* ref)
{ {
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
int32 result = GetNextDirents(entry, sizeof(longEntry), 1); int32 result = GetNextDirents(entry, sizeof(longEntry), 1);
if (result < 0) if (result < 0)
return result; return result;
+1 -1
View File
@@ -320,7 +320,7 @@ BNode::GetNextAttrName(char* buffer)
return B_FILE_ERROR; return B_FILE_ERROR;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1); ssize_t result = _kern_read_dir(fAttrFd, entry, sizeof(longEntry), 1);
if (result < 0) if (result < 0)
return result; return result;
+1 -1
View File
@@ -349,7 +349,7 @@ BQuery::GetNextRef(entry_ref* ref)
error = B_FILE_ERROR; error = B_FILE_ERROR;
if (error == B_OK) { if (error == B_OK) {
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
bool next = true; bool next = true;
while (error == B_OK && next) { while (error == B_OK && next) {
if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) { if (GetNextDirents(entry, sizeof(longEntry), 1) != 1) {
@@ -83,7 +83,7 @@ status_t
VirtualDirectoryEntryList::GetNextRef(entry_ref* ref) VirtualDirectoryEntryList::GetNextRef(entry_ref* ref)
{ {
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
int32 result = GetNextDirents(entry, sizeof(longEntry), 1); int32 result = GetNextDirents(entry, sizeof(longEntry), 1);
if (result < 0) if (result < 0)
return result; return result;
@@ -209,7 +209,7 @@ VirtualDirectoryPoseView::_EntryCreated(const BMessage* message)
return true; return true;
BPrivate::Storage::LongDirEntry longEntry; BPrivate::Storage::LongDirEntry longEntry;
struct dirent* entry = &longEntry.dirent; struct dirent* entry = longEntry.dirent();
while (directory.GetNextDirents(entry, sizeof(longEntry), 1) == 1) { while (directory.GetNextDirents(entry, sizeof(longEntry), 1) == 1) {
if (strcmp(entry->d_name, ".") != 0 if (strcmp(entry->d_name, ".") != 0
&& strcmp(entry->d_name, "..") != 0) { && strcmp(entry->d_name, "..") != 0) {
+13 -14
View File
@@ -42,10 +42,9 @@ using namespace boot;
struct __DIR { struct __DIR {
Directory* directory; Directory* directory;
void* cookie; void* cookie;
union {
dirent entry; char _direntBuffer[sizeof(dirent) + B_FILE_NAME_LENGTH + 1];
char nameBuffer[sizeof(dirent) + B_FILE_NAME_LENGTH - 1]; dirent* entry() { return (dirent*)_direntBuffer; }
};
}; };
@@ -1260,33 +1259,33 @@ readdir(DIR* dir)
for (;;) { for (;;) {
status_t error = dir->directory->GetNextEntry(dir->cookie, status_t error = dir->directory->GetNextEntry(dir->cookie,
dir->entry.d_name, B_FILE_NAME_LENGTH); dir->entry()->d_name, B_FILE_NAME_LENGTH);
if (error != B_OK) { if (error != B_OK) {
errno = error; errno = error;
return NULL; return NULL;
} }
dir->entry.d_pdev = 0; dir->entry()->d_pdev = 0;
// not supported // not supported
dir->entry.d_pino = dir->directory->Inode(); dir->entry()->d_pino = dir->directory->Inode();
dir->entry.d_dev = dir->entry.d_pdev; dir->entry()->d_dev = dir->entry()->d_pdev;
// not supported // not supported
if (strcmp(dir->entry.d_name, ".") == 0 if (strcmp(dir->entry()->d_name, ".") == 0
|| strcmp(dir->entry.d_name, "..") == 0) { || strcmp(dir->entry()->d_name, "..") == 0) {
// Note: That's obviously not correct for "..", but we can't // Note: That's obviously not correct for "..", but we can't
// retrieve that information. // retrieve that information.
dir->entry.d_ino = dir->entry.d_pino; dir->entry()->d_ino = dir->entry()->d_pino;
} else { } else {
Node* node = dir->directory->Lookup(dir->entry.d_name, false); Node* node = dir->directory->Lookup(dir->entry()->d_name, false);
if (node == NULL) if (node == NULL)
continue; continue;
dir->entry.d_ino = node->Inode(); dir->entry()->d_ino = node->Inode();
node->Release(); node->Release();
} }
return &dir->entry; return dir->entry();
} }
} }