starred/s3fs-fuse

Fork 0

mirror of https://github.com/s3fs-fuse/s3fs-fuse.git synced 2026-04-25 13:26:00 +03:00

[GH-ISSUE #1371] Organize and split large source files #735

New issue

Closed

opened 2026-03-04 01:48:18 +03:00 by kerem · 4 comments

kerem commented

2026-03-04 01:48:18 +03:00

Owner

Originally created by @ggtakec on GitHub (Aug 19, 2020).
Original GitHub issue: https://github.com/s3fs-fuse/s3fs-fuse/issues/1371

#1369 #1370

Details about issue

Each file in the s3fs source code is getting bigger and bigger.
I think it's about time to divide files by class, function and etc.

First, I would like to split the file without changing the logic and functionality.
(Thus, I want to make the split files as easy as possible, so we need to merge the existed PR quickly.)

If we do this, we may be a little more difficult when it comes to user support.
When we support issues, we may have a hard time because the log information to be attached is output with a file name different from the master file.
But I think it should be done now.

If you agree with this, I would like to work on file division as soon as possible.

And I have one more suggestion that is I would like to change the indentation of 2 space characters to 4 space with this split.
(I personally prefer tab, but s3fs uses space indenting, so I think the space character should be continued.)
Please give me your opinion about this.

Originally created by @ggtakec on GitHub (Aug 19, 2020). Original GitHub issue: https://github.com/s3fs-fuse/s3fs-fuse/issues/1371 #### Related Issues #1369 #1370 ### Details about issue Each file in the s3fs source code is getting bigger and bigger. I think it's about time to divide files by class, function and etc. First, I would like to split the file without changing the logic and functionality. (Thus, I want to make the split files as easy as possible, so we need to merge the existed PR quickly.) If we do this, we may be a little more difficult when it comes to user support. When we support issues, we may have a hard time because the log information to be attached is output with a file name different from the master file. But I think it should be done now. If you agree with this, I would like to work on file division as soon as possible. And I have one more suggestion that is I would like to change the indentation of 2 space characters to 4 space with this split. (I personally prefer tab, but s3fs uses space indenting, so I think the space character should be continued.) Please give me your opinion about this.

kerem closed this issue

2026-03-04 01:48:18 +03:00

kerem commented

2026-03-04 01:48:19 +03:00

Author

Owner

@gaul commented on GitHub (Aug 19, 2020):

Agreed that these files have grown too big:

$ wc -l src/*.cpp | sort -n | tail -5
  1597 src/s3fs_util.cpp
  3465 src/fdcache.cpp
  4901 src/curl.cpp
  5629 src/s3fs.cpp
 18905 total

Let's tackle the files in individual PRs. fdcache.cpp looks the easiest, putting FdEntity and PageList in separate files. curl.cpp and s3fs.cpp seem harder -- how do you propose organizing them?

Let's address the formatting in a separate issue. I would really like to use clang-tidy but s3fs lack of whitespace around braces makes it difficult:

https://bugs.llvm.org/show_bug.cgi?id=43144

@gaul commented on GitHub (Aug 19, 2020): Agreed that these files have grown too big: ``` $ wc -l src/*.cpp | sort -n | tail -5 1597 src/s3fs_util.cpp 3465 src/fdcache.cpp 4901 src/curl.cpp 5629 src/s3fs.cpp 18905 total ``` Let's tackle the files in individual PRs. `fdcache.cpp` looks the easiest, putting `FdEntity` and `PageList` in separate files. `curl.cpp` and `s3fs.cpp` seem harder -- how do you propose organizing them? Let's address the formatting in a separate issue. I would really like to use clang-tidy but s3fs lack of whitespace around braces makes it difficult: https://bugs.llvm.org/show_bug.cgi?id=43144

kerem commented

2026-03-04 01:48:19 +03:00

Author

Owner

@ggtakec commented on GitHub (Aug 19, 2020):

What about working with the following priorities at first time?
(As a condition, the logic is not changed in order not to embed defects.)

Divide header and code files that define multiple classes by class
Classify utility functions, macros, definitions, etc., divide them into appropriate files, and put them together
Fixed indent only (2 space -> 4 space)

First, I try to make this PR available on the weekend.

Next, I think s3fs.cpp and curl.cpp will be reviewed after it.
For example, the parsing part for options, the head request part, etc. will be able to be decomposed, and moved to another class(or a utility class will be created).
curl.cpp decomposes the class if possible for class inheritance, but I think it causes extra confusion.
Therefore, it is possible to group methods by function or attribute and divide them into files. (***_priv.cpp etc.)

Finally, it is better not to be in a format that is too bound by rules, but if it becomes more convenient with tools etc., I would like to organize it in that range.
So that, I think that we may be able to support clang-tidy.

@ggtakec commented on GitHub (Aug 19, 2020): What about working with the following priorities at first time? (As a condition, the logic is not changed in order not to embed defects.) - Divide header and code files that define multiple classes by class - Classify utility functions, macros, definitions, etc., divide them into appropriate files, and put them together - Fixed indent only (2 space -> 4 space) First, I try to make this PR available on the weekend. Next, I think s3fs.cpp and curl.cpp will be reviewed after it. For example, the parsing part for options, the head request part, etc. will be able to be decomposed, and moved to another class(or a utility class will be created). curl.cpp decomposes the class if possible for class inheritance, but I think it causes extra confusion. Therefore, it is possible to group methods by function or attribute and divide them into files. (***_priv.cpp etc.) Finally, it is better not to be in a format that is too bound by rules, but if it becomes more convenient with tools etc., I would like to organize it in that range. So that, I think that we may be able to support clang-tidy.

kerem commented

2026-03-04 01:48:19 +03:00

Author

Owner

@gaul commented on GitHub (Sep 12, 2020):

@ggtakec Is this fixed?

@gaul commented on GitHub (Sep 12, 2020): @ggtakec Is this fixed?

kerem commented

2026-03-04 01:48:19 +03:00

Author

Owner

@ggtakec commented on GitHub (Sep 13, 2020):

@gaul Yes, this is the end of this matter.
If there are other necessary corrections, I will post a PR separately.

@ggtakec commented on GitHub (Sep 13, 2020): @gaul Yes, this is the end of this matter. If there are other necessary corrections, I will post a PR separately.