Resort to lstat for FS not supporting dirent.d_type

clearlinux / swupd-server

Software update server (deprecated)

Other

13 stars 17 forks source link

Resort to lstat for FS not supporting dirent.d_type #25

Closed rojkov closed 7 years ago

rojkov commented 8 years ago

According to POSIX.1 only d_name and d_ino fields of struct dirent are standardized. d_type isn't always correctly set on file systems like XFS. In such cases it makes sense to resort to lstat(). Otherwise a user has hard time figuring out what's wrong with her setup.

Signed-off-by: Dmitry Rozhkov dmitry.rozhkov@linux.intel.com

bradTpeters commented 8 years ago

I think option 2 is indeed the best approach; please revise your patch and resubmit and I'll +1

rojkov commented 8 years ago

removed redundant call to populate_file_struct();
rebased and force pushed.

phmccarty commented 7 years ago

Looks good to me. +1

phmccarty commented 7 years ago

For the record:

In master, this change was reverted due to test suite failures. It caused file types to be calculated for Manifest.full, but not for bundle manifests.

phmccarty commented 7 years ago

The performance improvements from the earlier discussion on this PR are still worth considering.

In the interim, @tmarcu pushed 74c8f86 to address the d_type portability issue.

pohly commented 7 years ago

@phmccarty with performance improvements you mean removing the extra populate_file_struct()? Should this be tracked in a new issue?

phmccarty commented 7 years ago

@pohly Exactly. I opened #39 for tracking.