Skip to content

Fix #51 segfault #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2024
Merged

Fix #51 segfault #53

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cdf424
Try to fix #51
probonopd Aug 5, 2024
e6cddd4
Revert recent changes
probonopd Aug 5, 2024
39bfcff
Try to fix build_mount_point
probonopd Aug 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/runtime/runtime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,25 +862,26 @@ bool rm_recursive(const char* const path) {
return rv == 0;
}

void build_mount_point(char* mount_dir, const char* const argv0, char const* const temp_base, const size_t templen) {
void build_mount_point(char* mount_dir, const char* const argv0, const char* const temp_base, const size_t templen) {
const size_t maxnamelen = 6;
const size_t prefix_len = 8; // Length of "/.mount_"
const size_t suffix_len = 6; // Length of "XXXXXX"

// Create a modifiable copy of argv0
char argv0_copy[PATH_MAX]; // Ensure this is large enough for your use case
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please stop using PATH_MAX! Not only me but others have told you more than once that this variable's name is really misleading. One can allocate the buffer to the correct size directly in this case. (Or just use C++...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See, e.g., #38, which attempts to fix some of this mess...

Copy link
Member Author

@probonopd probonopd Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point taken, but at least with this change we don't get an immediate segfault like we had before.

(This whole world of pointers and buffers is not what I normally want to deal with. But here in the runtime we currently have to use C indeed at the moment.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime really needs way more love to fix some of those C-isms. I wish C had a real string concept. But then again, we wanted to rewrite the runtime in another language anyway. The main issue with that is that libfuse is not available for languages like Rust, at least nothing that would be maintained or remotely up to date. Languages like Go don't fix the real issues we have either. C++ would fix most of the complaints but including the STL causes the binary to grow size wise.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. So C it is. Just needs some love.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I have some hope for Rust using FFI. That'd still eliminate most of the issues on our side.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will asprintf() help in that case?
Also things like strlcpy() are currently
available in glibc.
Just my 2 cents.

strncpy(argv0_copy, argv0, sizeof(argv0_copy) - 1);
argv0_copy[sizeof(argv0_copy) - 1] = '\0'; // Ensure null termination

// need to copy argv0 as it's a const value, basename intends to modify it
char* argv0_copy = strdup(argv0);
char* path_basename = basename(argv0_copy);
free(argv0_copy);

size_t namelen = strlen(path_basename);
// limit length of tempdir name
// Limit length of tempdir name
if (namelen > maxnamelen) {
namelen = maxnamelen;
}

strcpy(mount_dir, temp_base);
strncpy(mount_dir + templen, "/.mount_", 8);
strncpy(mount_dir + templen + 8, path_basename, namelen);
strncpy(mount_dir + templen + 8 + namelen, "XXXXXX", 6);
mount_dir[templen + 8 + namelen + 6] = 0; // null terminate destination
// Ensure mount_dir is large enough before copying
snprintf(mount_dir, templen + prefix_len + namelen + suffix_len + 1, "%s/.mount_%.*sXXXXXX", temp_base, (int)namelen, path_basename);
}

int fusefs_main(int argc, char* argv[], void (* mounted)(void)) {
Expand Down
Loading