Resolve "ls broken on some systems"

Marc Vefrequested to merge
85-ls-broken-on-some-systems into master

Closes #85 (closed)

This is partly a revert to previous functionality which was removed during code maintenance, but so that the modified linux_dirent struct is only used in gkfs_functions.cpp.

Also some comments added.

Edited by Marc Vef

Merge request reports

include/client/gkfs_functions.hpp

+4 −4
Changes for include/client/gkfs_functions.hpp: 4 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -19,8 +19,8 @@ 


struct statfs;

struct statvfs;

struct dirent;

struct dirent64;

struct linux_dirent;

struct linux_dirent64;



namespace gkfs {

namespace syscall {
@@ -79,9 +79,9 @@ ssize_t gkfs_read(int fd, void* buf, size_t count); 


int gkfs_opendir(const std::string& path);



int gkfs_getdents(unsigned int fd, struct dirent* dirp, unsigned int count);

int gkfs_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count);



int gkfs_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count);

int gkfs_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count);



int gkfs_rmdir(const std::string& path);

include/client/hooks.hpp

+4 −4
Changes for include/client/hooks.hpp: 4 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -18,8 +18,8 @@ 
#include <fcntl.h>



struct statfs;

struct dirent;

struct dirent64;

struct linux_dirent;

struct linux_dirent64;



namespace gkfs {

namespace hook {
@@ -69,9 +69,9 @@ int hook_dup2(unsigned int oldfd, unsigned int newfd); 


int hook_dup3(unsigned int oldfd, unsigned int newfd, int flags);



int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count);

int hook_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count);



int hook_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count);

int hook_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count);



int hook_mkdirat(int dirfd, const char* cpath, mode_t mode);

include/client/open_dir.hpp

+0 −2
Changes for include/client/open_dir.hpp: 0 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -18,8 +18,6 @@ 
#include <string>

#include <vector>



#include <dirent.h>



#include <client/open_file_map.hpp>



namespace gkfs {

src/client/gkfs_functions.cpp

+62 −20
Changes for src/client/gkfs_functions.cpp: 62 added lines, 20 removed lines.
Original line number Diff line number Diff line
@@ -24,16 +24,41 @@ 
#include <global/path_util.hpp>



extern "C" {

#include <dirent.h> // used for file types in the getdents{,64}() functions

#include <linux/kernel.h> // used for definition of alignment macros

#include <sys/statfs.h>

#include <sys/statvfs.h>

#include <dirent.h>

}



#define __ALIGN_KERNEL_MASK(x, mask)    (((x) + (mask)) & ~(mask))

#define __ALIGN_KERNEL(x, a)            __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)

using namespace std;



/*

 * Macro used within getdents{,64} functions.

 * __ALIGN_KERNEL defined in linux/kernel.h

 */

#define ALIGN(x, a)                     __ALIGN_KERNEL((x), (a))



using namespace std;

/*

 * linux_dirent is used in getdents() but is privately defined in the linux kernel: fs/readdir.c.

 */

struct linux_dirent {

    unsigned long d_ino;

    unsigned long d_off;

    unsigned short d_reclen;

    char d_name[1];

};

/*

 * linux_dirent64 is used in getdents64() and defined in the linux kernel: include/linux/dirent.h.

 * However, it is not part of the kernel-headers and cannot be imported.

 */

struct linux_dirent64 {

    uint64_t d_ino;

    int64_t d_off;

    unsigned short d_reclen;

    unsigned char d_type;

    char d_name[1]; // originally `char d_name[0]` in kernel, but ISO C++ forbids zero-size array 'd_name'

};





namespace {
@@ -505,9 +530,10 @@ int gkfs_rmdir(const std::string& path) { 
}



int gkfs_getdents(unsigned int fd,

                  struct dirent* dirp,

                  struct linux_dirent* dirp,

                  unsigned int count) {



    // Get opendir object (content was downloaded with opendir() call)

    auto open_dir = CTX->file_map()->get_dir(fd);

    if (open_dir == nullptr) {

        //Cast did not succeeded: open_file is a regular file
@@ -515,24 +541,32 @@ int gkfs_getdents(unsigned int fd, 
        return -1;

    }



    // get directory position of which entries to return

    auto pos = open_dir->pos();

    if (pos >= open_dir->size()) {

        return 0;

    }



    unsigned int written = 0;

    struct dirent* current_dirp = nullptr;

    struct linux_dirent* current_dirp = nullptr;

    while (pos < open_dir->size()) {

        gkfs::filemap::DirEntry de = open_dir->getdent(pos);

        // get dentry fir current position

        auto de = open_dir->getdent(pos);

        /*

         * Calculate the total dentry size within the kernel struct `linux_dirent` depending on the file name size.

         * The size is then aligned to the size of `long` boundary.

         * This line was originally defined in the linux kernel: fs/readdir.c in function filldir():

         * int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2, sizeof(long));

         * However, since d_name is null-terminated and de.name().size() does not include space

         * for the null-terminator, we add 1. Thus, + 3 in total.

         */

        auto total_size = ALIGN(offsetof(

                                        struct dirent, d_name) +

                                        de.name().size() + 3, sizeof(long));

                                        struct linux_dirent, d_name) + de.name().size() + 3, sizeof(long));

        if (total_size > (count - written)) {

            //no enough space left on user buffer to insert next dirent

            break;

        }

        current_dirp = reinterpret_cast<struct dirent*>(

                reinterpret_cast<char*>(dirp) + written);

        current_dirp = reinterpret_cast<struct linux_dirent*>(reinterpret_cast<char*>(dirp) + written);

        current_dirp->d_ino = std::hash<std::string>()(

                open_dir->path() + "/" + de.name());
@@ -552,13 +586,14 @@ int gkfs_getdents(unsigned int fd, 
        errno = EINVAL;

        return -1;

    }

    // set directory position for next getdents() call

    open_dir->pos(pos);

    return written;

}





int gkfs_getdents64(unsigned int fd,

                    struct dirent64* dirp,

                    struct linux_dirent64* dirp,

                    unsigned int count) {



    auto open_dir = CTX->file_map()->get_dir(fd);
@@ -567,25 +602,32 @@ int gkfs_getdents64(unsigned int fd, 
        errno = EBADF;

        return -1;

    }



    auto pos = open_dir->pos();

    if (pos >= open_dir->size()) {

        return 0;

    }



    unsigned int written = 0;

    struct dirent64* current_dirp = nullptr;

    struct linux_dirent64* current_dirp = nullptr;

    while (pos < open_dir->size()) {

        gkfs::filemap::DirEntry de = open_dir->getdent(pos);

        auto de = open_dir->getdent(pos);

        /*

         * Calculate the total dentry size within the kernel struct `linux_dirent` depending on the file name size.

         * The size is then aligned to the size of `long` boundary.

         *

         * This line was originally defined in the linux kernel: fs/readdir.c in function filldir64():

         * int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1, sizeof(u64));

         * We keep + 1 because:

         * Since d_name is null-terminated and de.name().size() does not include space

         * for the null-terminator, we add 1. Since d_name in our `struct linux_dirent64` definition

         * is not a zero-size array (as opposed to the kernel version), we subtract 1. Thus, it stays + 1.

         */

        auto total_size = ALIGN(offsetof(

                                        struct dirent64, d_name) +

                                        de.name().size() + 3, sizeof(long));

                                        struct linux_dirent64, d_name) + de.name().size() + 1, sizeof(uint64_t));

        if (total_size > (count - written)) {

            //no enough space left on user buffer to insert next dirent

            break;

        }

        current_dirp = reinterpret_cast<struct dirent64*>(

                reinterpret_cast<char*>(dirp) + written);

        current_dirp = reinterpret_cast<struct linux_dirent64*>(reinterpret_cast<char*>(dirp) + written);

        current_dirp->d_ino = std::hash<std::string>()(

                open_dir->path() + "/" + de.name());

src/client/hooks.cpp

+2 −3
Changes for src/client/hooks.cpp: 2 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ 


extern "C" {

#include <libsyscall_intercept_hook_point.h>

#include <dirent.h>

#include <fcntl.h>

#include <sys/stat.h>

#include <sys/statfs.h>
@@ -413,7 +412,7 @@ int hook_dup3(unsigned int oldfd, unsigned int newfd, int flags) { 
    return syscall_no_intercept(SYS_dup3, oldfd, newfd, flags);

}



int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count) {

int hook_getdents(unsigned int fd, struct linux_dirent* dirp, unsigned int count) {



    LOG(DEBUG, "{}() called with fd: {}, dirp: {}, count: {}",

        __func__, fd, fmt::ptr(dirp), count);
@@ -425,7 +424,7 @@ int hook_getdents(unsigned int fd, struct dirent* dirp, unsigned int count) { 
}





int hook_getdents64(unsigned int fd, struct dirent64* dirp, unsigned int count) {

int hook_getdents64(unsigned int fd, struct linux_dirent64* dirp, unsigned int count) {



    LOG(DEBUG, "{}() called with fd: {}, dirp: {}, count: {}",

        __func__, fd, fmt::ptr(dirp), count);