lib/*fs, lib/vfscore: Add proper dirent/dirent64 structure usage

[StefanJum]

Unikraft defines the dirent structure as identical to dirent64.
This works fine if not using Unikraft in binary compatibility mode,
since we have total control over the internal functions/structures that
are used.

However, if we use Unikraft in binary compatibility mode, we want to use
Linux dirent and dirent64 structures, in order to maintain compatibility
with older libc versions that do not use the *64 calls by default.
Since the filesystems READDIR vop uses dirent64, we will convert to a
dirent structure within the readdir_r call.

You can see the issue happening in #919
In order to test it with the app-elfloader, you can use the ls executable from the dynamic-apps repository, which uses an old libc version that calls getdents instead of getdents64.

Prerequisite checklist

• Read the contribution guidelines regarding submitting new changes to the project;
• Tested your changes against relevant architectures and platforms;
• Ran the checkpatch.uk on your commit series before opening this PR;
• Updated relevant documentation.

[mogasergiu]

Just a more general comment, but would it be a good idea to use unlikely's on error path's here as well? 🤔

[StefanJum]

Done @skuenzer, I moved the config option in vfscore, it will get enabled by default if the syscall handler is enabled.

Also added the unlikely's as @mogasergiu suggested .

[RaduNichita]

Thanks a lot for providing the fix, @StefanJum. I have provided some nitpicks.

I will approve the PR after that! 💪

[StefanJum]

Done @RaduNichita, thank you.

[RaduNichita]

All good now. Thanks, @StefanJum! 💯

Reviewed-by: Radu Nichita radunichita99@gmail.com

[mogasergiu]

Looks cool to me, some comments for your notification box 📫.

[StefanJum]

All done @skuenzer, sorry for the force-push spam, there were some checkpatch and ci/cd issues.

[StefanJum]

WARNING:LINE_SPACING: Missing a blank line after declarations
#233: FILE: lib/vfscore/main.c:1080:
int (*cmp)(const struct dirent **, const struct dirent **))
__attribute__((alias("scandir64")));

The checkpatch is getting confused by the function pointer argument here 😆

[mogasergiu]

WARNING:LINE_SPACING: Missing a blank line after declarations
#233: FILE: lib/vfscore/main.c:1080:
int (*cmp)(const struct dirent **, const struct dirent **))
attribute((alias("scandir64")));

The checkpatch is getting confused by the function pointer argument here laughing

The whole scandir64 function looks like a big checkpatch violation to be honest 😤. Maybe we could rewrite it better 🤔.

[StefanJum]

The whole scandir64 function looks like a big checkpatch violation to be honest 😤. Maybe we could rewrite it better 🤔.

Yes, it's absolutely hideous, I just copied the scandir function. I'll try to do a rewrite if I catch some time.

[skuenzer]

⚠️ Checkpatch passed with warnings

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered warnings:
SHA commit checkpatch
cfc5c4c lib/vfscore: Remove readdir_r result from TLS ✅
093be54 lib/vfscore: Add non-largefile variant for dirent ⚠️

Truncated logs starting from first warning 093be54:

WARNING:LONG_LINE: line over 80 characters
#68: FILE: lib/nolibc/include/dirent.h:22:
+#if !defined(CONFIG_LIBVFSCORE_NONLARGEFILE) || (CONFIG_LIBVFSCORE_NONLARGEFILE <= 0)

WARNING:LINE_SPACING: Missing a blank line after declarations
#233: FILE: lib/vfscore/main.c:1080:
+	int (*cmp)(const struct dirent **, const struct dirent **))
+	__attribute__((alias("scandir64")));

total: 0 errors, 2 warnings, 377 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0002-lib-vfscore-Add-non-largefile-variant-for-dirent.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

Because I am seeing this here:
You do not need to do

#if !defined(CONFIG_LIBVFSCORE_NONLARGEFILE) || (CONFIG_LIBVFSCORE_NONLARGEFILE <= 0)

the following achieves exactly the same (except it does not check for < 0, but it is fine to assume a CONFIG to be set to 0(and undefined) or 1):

+#if !CONFIG_LIBVFSCORE_NONLARGEFILE

The preprocessor handles the case of being not defined correctly.

[StefanJum]

The preprocessor handles the case of being not defined correctly.

I did not know that 👀 Pushed the change.

[StefanJum]

Updated @skuenzer, thanks.

[unikraft-bot]

⚠️ Checkpatch passed with warnings

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered warnings:

SHA	commit	checkpatch
cfc5c4c	lib/vfscore: Remove readdir_r result from TLS	✅
fe58a9a	lib/vfscore: Add non-largefile variant for dirent	⚠️

Truncated logs starting from first warning fe58a9a:

WARNING:LINE_SPACING: Missing a blank line after declarations
#231: FILE: lib/vfscore/main.c:1080:
+	int (*cmp)(const struct dirent **, const struct dirent **))
+	__attribute__((alias("scandir64")));

total: 0 errors, 1 warnings, 389 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0002-lib-vfscore-Add-non-largefile-variant-for-dirent.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

---

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

[skuenzer]

Thanks a lot for your work!

Reviewed-by: Simon Kuenzer simon@unikraft.io
Approved-by: Simon Kuenzer simon@unikraft.io