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

Add "." and ".." in directory listings #139

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DrDaveD
Copy link
Collaborator

@DrDaveD DrDaveD commented Oct 8, 2024

This is a first attempt at

I'm sure it is incomplete but it does work to make ls -a show . and .. files.

@vasi Can you give me some hints on what else is needed? I didn't know what to fill in the inode and inode_number fields but it doesn't seem to make a difference for ls -a and ls -al. Also I would think that some of the other functions in dir.c that deal with offsets will need some changes but I don't really know what changes.

I came up with the idea of adding fake offset indexes from the corresponding squashfs kernel code.

@DrDaveD DrDaveD marked this pull request as draft October 8, 2024 20:30
@vasi
Copy link
Owner

vasi commented Oct 9, 2024

Thanks! I'm on vacation now, so I'll only be able to look at this in a couple of weeks. Sorry for the delay.

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Oct 23, 2024

@vasi are you back?

@vasi
Copy link
Owner

vasi commented Oct 24, 2024

Hi @DrDaveD , thanks for the ping. I just arrived home, I'll try to look at this soon. Just trying to unpack, figure out where my lost luggage is, etc!

@vasi
Copy link
Owner

vasi commented Oct 24, 2024

Gonna do a quick pass now.

Copy link
Owner

@vasi vasi left a comment

Choose a reason for hiding this comment

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

So the fundamental issue here is what we're trying to change the semantics of:

  1. The squashfuse_ll/_hl FUSE programs, or
  2. The squashfuse internal representation (and/or libsquashfuse)

What you've done here is a bit in the middle, yah? We're changing the meaning of sqfs_dir_entry->offset (and ->next_offset), but we're not changing the meaning of sqfs_traverse_next(). We should go either all-in on 1 or 2.

Advantages for each option:

1. Changing FUSE programs only

  • Existing programs that use libsquashfuse keep working.
  • No need to fix sqfs_dir_ff_offset() and friends to use our new definition of offset.
  • Doesn't make non-UNIX drivers awkward, eg: if someone wanted to implement a Windows filesystem for squashfuse.

2. Changing libsquashfuse semantics

  • We only have to implement ./.. once in the library code, not for each of low-level and high-level.

if (entry->offset < 2) {
/* offsets 0 and 1 are '.' and '..' which are not stored */
entry->type = SQUASHFS_DIR_TYPE;
if (entry->name != NULL) {
Copy link
Owner

Choose a reason for hiding this comment

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

Minor nit: This probably looks a lot nicer using strncpy() (or similar) than doing character-by-character modification. Similarly below in traverse.c, using strcmp().

@vasi
Copy link
Owner

vasi commented Oct 24, 2024

Also thank you so much for your patience, I'm sorry for the bad vacation timing.

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Oct 24, 2024

Ok, I didn't understand which files were considered part of libsquashfuse. It sounds there are more advantages to changing it only in the FUSE programs. Maybe the duplicate changes in hl/ll can be kept to a minimum via shared function(s).

It looks like both of the source files I changed are part of libsquashfuse. So if I avoided those, it would be the readdir functions in ll.c & hl.c that I should be modifying instead? I guess I would need to add an additional variable in the ll and hl structures to indicate whether it's time to return ., .., or call sqfs_dir_next.

@vasi
Copy link
Owner

vasi commented Oct 24, 2024

I think you'd need to change sqfs_hl_op_readdir/sqfs_ll_op_readdir to:

  • Translate between "UNIX offset" and "libsquashfuse offset". You'd want to translate UNIX -> libsquashfuse for the input parameter, and libsquashfuse -> UNIX for the offsets returned via fuse_fill_dir_t/sqfs_ll_add_direntry.
  • When given an offset low enough, output the dotfiles before doing the real work.

Any common functions you build could go in fuseprivate.c, to minimize duplicate work.

Oh, and the last thing is that I have no idea whether we care about the inode values for the dot entries. We should try to test whether things like ls mnt/foo/.. works correctly with our change,

@DrDaveD
Copy link
Collaborator Author

DrDaveD commented Oct 25, 2024

Thanks, that seems quite clear.

I'm quite sure that .. and . as part of file paths already work, independent of what shows up in a directory listing, but I'll make sure they still work when I am finished.

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

Successfully merging this pull request may close these issues.

getdents does not list . and .. files on squashfuse-mounted filesystems
2 participants