[LibOS] Enable readdir to list directories with many entries

chroot_readdir() tried to allocate a buffer big enough to hold all
entries, by calling DkStreamRead until it succeeded (increasing buffer
size otherwise). However, DkStreamRead could return partial results,
which was never handled (partial result means here it could return
just some objects, not a part of individual object). This commit fixes
this issue and refactors this function.

LibOS/shim/include/shim_handle.h

@@ -23,6 +23,8 @@
 #ifndef _SHIM_HANDLE_H_
 #define _SHIM_HANDLE_H_
 
+#include <stdalign.h>
+
 #include <shim_types.h>
 #include <shim_defs.h>
 #include <shim_sysv.h>
@@ -212,6 +214,16 @@ struct shim_dirent {
     char                 name[];       /* File name (null-terminated) */
 };
 
+#define SHIM_DIRENT_SIZE offsetof(struct shim_dirent, name)
+#define SHIM_DIRENT_ALIGNMENT alignof(struct shim_dirent)
+/* Size of struct shim_dirent instance together with alignment,
+ * which might be different depending on the length of the name field */
+#define SHIM_DIRENT_ALIGNED_SIZE(len) ( \
+                (SHIM_DIRENT_SIZE \
+                    + (len) \
+                    + SHIM_DIRENT_ALIGNMENT - 1) \
+            / SHIM_DIRENT_ALIGNMENT * SHIM_DIRENT_ALIGNMENT)
+
 struct shim_dir_handle {
     int offset;
     struct shim_dentry * dotdot;

LibOS/shim/src/fs/chroot/fs.c

@@ -1004,133 +1004,133 @@ static int chroot_dput (struct shim_dentry * dent)
     return 0;
 }
 
-static int chroot_readdir (struct shim_dentry * dent,
-                           struct shim_dirent ** dirent)
-{
-    struct shim_file_data * data;
-    int ret;
+static int chroot_readdir(struct shim_dentry* dent, struct shim_dirent** dirent) {
+    struct shim_file_data* data = NULL;
+    int ret = 0;
+    PAL_HANDLE pal_hdl = NULL;
+    size_t buf_size = MAX_PATH,
+           dirent_buf_size = 0;
+    char* buf = NULL;
+    char* dirent_buf = NULL;
 
     if ((ret = try_create_data(dent, NULL, 0, &data)) < 0)
         return ret;
 
     chroot_update_ino(dent);
-    const char * uri = qstrgetstr(&data->host_uri);
+
+    const char* uri = qstrgetstr(&data->host_uri);
     assert(strpartcmp_static(uri, "dir:"));
 
-    PAL_HANDLE pal_hdl = DkStreamOpen(uri, PAL_ACCESS_RDONLY, 0, 0, 0);
+    pal_hdl = DkStreamOpen(uri, PAL_ACCESS_RDONLY, 0, 0, 0);
     if (!pal_hdl)
         return -PAL_ERRNO;
 
-    size_t buf_size = MAX_PATH, bytes = 0;
-    char * buf = malloc(buf_size);
+    buf = malloc(buf_size);
     if (!buf) {
         ret = -ENOMEM;
-        goto out_hdl;
+        goto out;
     }
 
-    /*
-     * Try to read the directory list from the host. DkStreamRead
-     * does not accept offset for directory listing. Therefore, we retry
-     * several times if the buffer is not large enough.
-     */
-retry_read:
-    bytes = DkStreamRead(pal_hdl, 0, buf_size, buf, NULL, 0);
-    if (!bytes) {
-        ret = 0;
-        if (PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM)
-            goto out;
-
-        if (PAL_NATIVE_ERRNO == PAL_ERROR_OVERFLOW) {
-            char * new_buf = malloc(buf_size * 2);
-            if (!new_buf) {
-                ret = -ENOMEM;
-                goto out;
+    while (1) {
+        /* DkStreamRead for directory will return as many entries as fits into the buffer. */
+        size_t bytes = DkStreamRead(pal_hdl, 0, buf_size, buf, NULL, 0);
+        if (!bytes) {
+            if (PAL_NATIVE_ERRNO == PAL_ERROR_ENDOFSTREAM) {
+                /* End of directory listing */
+                ret = 0;
+                break;
             }
 
-            free(buf);
-            buf_size *= 2;
-            buf = new_buf;
-            goto retry_read;
+            ret = -PAL_ERRNO;
+            goto out;
         }
-
-        ret = -PAL_ERRNO;
-        goto out;
-    }
-
-    /* Now emitting the dirent data */
-    size_t dbuf_size = MAX_PATH;
-    struct shim_dirent * dbuf = malloc(dbuf_size);
-    if (!dbuf)
-        goto out;
-
-    struct shim_dirent * d = dbuf, ** last = NULL;
-    char * b = buf, * next_b;
-    int blen;
-
-    /* Scanning the directory names in the buffer */
-    while (b < buf + bytes) {
-        blen = strlen(b);
-        next_b = b + blen + 1;
-        bool isdir = false;
-
-        /* The PAL convention: if the name is ended with "/",
-           it is a directory. */
-        if (b[blen - 1] == '/') {
-            isdir = true;
-            b[blen - 1] = 0;
-            blen--;
+        /* Last entry must be null-terminated */
+        assert(buf[bytes - 1] == '\0');
+
+        size_t dirent_cur_off = dirent_buf_size;
+        /* Calculate needed buffer size */
+        size_t len = buf[0] != '\0' ? 1 : 0;
+        for (size_t i = 1; i < bytes; i++) {
+            if (buf[i] == '\0') {
+                /* The PAL convention: if a name ends with '/', it is a directory.
+                 * struct shim_dirent has a field for a type, hence trailing slash
+                 * can be safely discarded. */
+                if (buf[i - 1] == '/') {
+                    len--;
+                }
+                dirent_buf_size += SHIM_DIRENT_ALIGNED_SIZE(len + 1);
+                len = 0;
+            } else {
+                len++;
+            }
         }
 
-        /* Populating a dirent */
-        int dsize = sizeof(struct shim_dirent) + blen + 1;
-
-        /* dbuf is not large enough, reallocate the dirent buffer */
-        if ((void *) d + dsize > (void *) dbuf + dbuf_size) {
-            int newsize = dbuf_size * 2;
-            while ((void *) d + dsize > (void *) dbuf + newsize)
-                newsize *= 2;
-
-            struct shim_dirent * new_dbuf = malloc(newsize);
-            if (!new_dbuf) {
-                ret = -ENOMEM;
-                free(dbuf);
-                goto out;
+        /* TODO: If realloc gets enabled delete following and uncomment rest */
+        char* tmp = malloc(dirent_buf_size);
+        if (!tmp) {
+            ret = -ENOMEM;
+            goto out;
+        }
+        memcpy(tmp, dirent_buf, dirent_cur_off);
+        free(dirent_buf);
+        dirent_buf = tmp;
+        /*
+        dirent_buf = realloc(dirent_buf, dirent_buf_size);
+        if (!dirent_buf) {
+            ret = -ENOMEM;
+            goto out;
+        }
+        */
+
+        size_t i = 0;
+        while (i < bytes) {
+            char* name = buf + i;
+            size_t len = strnlen(name, bytes - i);
+            i += len + 1;
+            bool is_dir = false;
+
+            /* Skipping trailing slash - explained above */
+            if (name[len - 1] == '/') {
+                is_dir = true;
+                name[--len] = '\0';
             }
 
-            memcpy(new_dbuf, dbuf, (void *) d - (void *) dbuf);
-            struct shim_dirent * d1 = new_dbuf;
-            struct shim_dirent * d2 = dbuf;
-            while (d2 != d) {
-                d1->next = (void *) d1 + ((void *) d2->next - (void *) d2);
-                d1 = d1->next;
-                d2 = d2->next;
-            }
+            struct shim_dirent* dptr = (struct shim_dirent*)(dirent_buf + dirent_cur_off);
+            dptr->ino = rehash_name(dent->ino, name, len);
+            dptr->type = is_dir ? LINUX_DT_DIR : LINUX_DT_REG;
+            memcpy(dptr->name, name, len + 1);
 
-            free(dbuf);
-            dbuf = new_dbuf;
-            d = d1;
-            dbuf_size = newsize;
+            dirent_cur_off += SHIM_DIRENT_ALIGNED_SIZE(len + 1);
         }
+    }
 
-        /* Fill up the dirent buffer */
-        HASHTYPE hash = rehash_name(dent->ino, b, blen);
-
-        d->next = (void *) (d + 1) + blen + 1;
-        d->ino = hash;
-        d->type = isdir ? LINUX_DT_DIR : LINUX_DT_REG;
-        memcpy(d->name, b, blen + 1);
+    *dirent = (struct shim_dirent*)dirent_buf;
 
-        b = next_b;
-        last = &d->next;
-        d = d->next;
+    /*
+     * Fix next field of struct shim_dirent to point to the next entry.
+     * Since all entries are assumed to come from single allocation
+     * (as free gets called just on the head of this list) this should have
+     * been just entry size instead of a pointer (and probably needs to be
+     * rewritten as such one day).
+     */
+    struct shim_dirent** last = NULL;
+    for (size_t dirent_cur_off = 0; dirent_cur_off < dirent_buf_size; ) {
+        struct shim_dirent* dptr = (struct shim_dirent*)(dirent_buf + dirent_cur_off);
+        size_t len = SHIM_DIRENT_ALIGNED_SIZE(strlen(dptr->name) + 1);
+        dptr->next = (struct shim_dirent*)(dirent_buf + dirent_cur_off + len);
+        last = &dptr->next;
+        dirent_cur_off += len;
+    }
+    if (last) {
+        *last = NULL;
     }
-
-    *last = NULL;
-    *dirent = dbuf;
 
 out:
+    /* Need to free output buffer if error is returned */
+    if (ret) {
+        free(dirent_buf);
+    }
     free(buf);
-out_hdl:
     DkObjectClose(pal_hdl);
     return ret;
 }