Skip to content
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.

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

odd handling of image_mountdir #359

Open
allisonkarlitskaya opened this issue Sep 24, 2024 · 1 comment
Open

odd handling of image_mountdir #359

allisonkarlitskaya opened this issue Sep 24, 2024 · 1 comment

Comments

@allisonkarlitskaya
Copy link
Collaborator

I've been spending a lot of time in lcfs_mount lately and there are a few weird issues around options->image_mountdir.

  • this is specified by the user as a const char * but we internally cast it to char * so that we can pass it to some functions which require it to be char *. But those functions don't modify it and can be changed to consume const char * without issue.

  • in the normal exit path (error or not error) we check that we were the ones that created the temporary directory before we remove it:

	if (created_tmpdir) {
		rmdir(imagemount);
        }
  • but we have an early exit path which removes it unconditionally (if we failed to mount the erofs)
	if (err < 0) {
		rmdir(imagemount);
		return err;
	}
  • but maybe we just want to eliminate this feature entirely. It's used by nothing else inside of composefs itself. ostree uses it, though, so maybe we decide to keep it....

  • but in that case, we should definitely improve the const-correctness issues and also fix the bug where we rmdir() the user's directory in some error cases but not others

  • all of this goes a little bit towards how complicated the error handling in C in libcomposefs has become....

@cgwalters
Copy link
Contributor

user's directory in some error cases but not others

Agree

. It's used by nothing else inside of composefs itself. ostree uses it, though, so maybe we decide to keep it....

Yeah can't break ostree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants