sysprog21 / simplefs

A simple native file system for Linux kernel
Other
362 stars 91 forks source link

Ensure superblock always fits SIMPLEFS_BLOCK_SIZE #49

Closed HotMercury closed 4 months ago

HotMercury commented 4 months ago

This commit ensures that the padding size always matches the block size defined by SIMPLEFS_BLOCK_SIZE.

jserv commented 4 months ago

In the commit message, instead of stating that "This commit ensures...," you should explain the issues or limitations present in the current implementation, such as poor cache locality. Additionally, if the proposed changes aim to enhance performance and/or improve the memory layout, utilize the term "improve" in the git commit message to accurately reflect the intended improvements.

jserv commented 4 months ago

_Static_assert is not a macro. It is a keyword in C11.

jserv commented 4 months ago

Consider the following changes:

--- a/mkfs.c
+++ b/mkfs.c
@@ -17,6 +17,8 @@ struct superblock {
     };
 };

+_Static_assert(sizeof(struct superblock) == SIMPLEFS_BLOCK_SIZE);
+
 /**
  * DIV_ROUND_UP - round up a division
  * @n: dividend
@@ -239,7 +241,6 @@ static int write_data_blocks(int fd, struct superblock *sb)

 int main(int argc, char **argv)
 {
-    _Static_assert(sizeof(struct superblock) == SIMPLEFS_BLOCK_SIZE);
     if (argc != 2) {
         fprintf(stderr, "Usage: %s disk\n", argv[0]);
         return EXIT_FAILURE;

In git commit message, you should explain why union does matter.

jserv commented 4 months ago

Thank @HotMercury for contributing!