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

tinydir_readfile returns -1 when file is a broken link #63

Open
asv22 opened this issue Feb 7, 2019 · 13 comments
Open

tinydir_readfile returns -1 when file is a broken link #63

asv22 opened this issue Feb 7, 2019 · 13 comments

Comments

@asv22
Copy link

asv22 commented Feb 7, 2019

I.e. a file, which you could create with

ln -s /broken/link .

In that case iterate_sample.c shows directory listing only partially. And random_access_sample.c fails to open a directory at line 8.

tinydir_readfile finishes at line 559 when processing a broken link

My OS is Linux.

@lautis0503
Copy link
Collaborator

@asv22 Try the latest commit to see if it's fixed.

@asv22
Copy link
Author

asv22 commented Feb 10, 2019

Thanks, for me it works great now.
It's worth noting that after the commit behavior of the library changed in this way:

before

If we have a link to an existing regular file, then file.is_dir=0, file.is_reg=1
If we have a link to an existing directory, then file.is_dir=1, file.is_reg=0

now

If we have a link to an existing regular file, then file.is_dir=0, file.is_reg=0
If we have a link to an existing directory, then file.is_dir=0, file.is_reg=0

@cxong
Copy link
Owner

cxong commented Feb 11, 2019

interesting; I guess to find the properties of the link target, you need stat anyway.
I think we need to make a choice about how to support links

  • if we pretend they are just real files then the existing behaviour is correct and we should update the samples to not break but continue when encountering broken links.
  • If we show links we'll need to add a is_link attribute, and either
    • provide a readlink wrapper to show the path of the target, or
    • call stat again to see if the target is a file/folder, possibly recursively since links can target other links.

@lautis0503 lautis0503 reopened this Feb 11, 2019
@lautis0503
Copy link
Collaborator

The second choice i will make.
Treat link files as something special and add a is_link attribute as in POSIX.1-2001.
is_dir and is_file should be 0.
We can provide new wrapper for read link files.

@wojdyr
Copy link
Contributor

wojdyr commented Oct 29, 2021

We had a related problem on Mac. The ifdefs before lstat enable it on Linux, but apparently not on macOS (which has lstat). We solved it by adding there

|| (defined __APPLE__)

@lautis0503
Copy link
Collaborator

I searched for manuals, and found lstat() is available in 4.2BSD.
Add the following for OS X and *BSD:

|| (defined__APPLE__) && (defined__MACH__)
|| (defined BSD)

lautis0503 added a commit that referenced this issue Nov 11, 2021
@lautis0503
Copy link
Collaborator

Sorry for late pull request #70. I'll merge it unless it doesn't work fine for you.

@ccbenny
Copy link

ccbenny commented Sep 23, 2022

Problems with symbolic links bit me recently in Tcl, which bundles Tinydir and Minizip, see https://core.tcl-lang.org/tcl/tktview/22ab2ae64ab2dd813719a985a6f125631ab1ba74 . I'd like this:

Treat link files as something special and add a is_link attribute as in POSIX.1-2001.
is_dir and is_file should be 0.
We can provide new wrapper for read link files.

Is that still the plan? It could be used in Minizip.

@lautis0503
Copy link
Collaborator

To be honest, we (at least myself) had no such thing called plan on this project. We will appear when any bugs or issues are found, because:

We have no ability to kill the bug, but the ability to kill the ones who raise the bug.

Just kidding.

I, myself, had never expected the project being used in any serious programs. (@cxong seems to use it in game, and I use it in personal project.) This is why when I made a dirty fix for this issue three years ago, although the API change is recognized, yet no actions were made to either fix the issue perfectly or prompt users to be aware of this API change. (If we used Semantic Versioning, the major version number should be increased.) Although that was a dirty fix, simply reverting the commit is another dirty fix, which make the original bug appear again.

More to say, in my opinion, if you want to use some program seriously, complete test suites or units should be used to make sure it all works. And this project is surely lack of that.

Thanks for your reporting, I would recently try to make this issue solved perfectly.

@cxong
Copy link
Owner

cxong commented Sep 25, 2022

I think an easy partial fix is to add the is_link property, and leave it to users what to do with it
I'm not sure what happens if a link points to another link, and if you can have loops in links, and how to handle that elegantly.

@lautis0503
Copy link
Collaborator

We may also add a readlink procedure, which read the destination that link file points to, with parameters similar to readfile.
With is_link and readlink, users could easily write down code like while is_link, readlink again.
One thing to consider, MSYS/MSYS2/Cygwin/WSL are Linux (or POSIX?) layers upon the Windows system, would they process link file differently?

@lautis0503
Copy link
Collaborator

According to man pages of ln(1) of UNIX v7 and 4BSD

A hard link to a file is indistinguishable from the original directory entry; any changes to a file are effective independent of the name used to reference the file.

This means the hard links don't matter. We need to process the symbolic links.

According to man page of ln(1p) of POSIX.1-2008:

  1. Switchs can be used to specify whether the source symbolic links are dereferenced when creating hard links.
  2. Creating symbolic links doesn't care about whether the source files are symbolic links.

According to The GNU C Library Reference Manual, for version 2.36:

The macro MAXSYMLINKS specifies how many symlinks some function will follow before returning ELOOP.

@lautis0503
Copy link
Collaborator

lautis0503 commented Jan 3, 2023

According to Microsoft Open Specifications:

The .LNK file is called Shell Link (.LNK) Binary File Format. This is a file for shell, with no relation to UNIX link.

In Microsoft Windows, there is mklink command which could create Hard Links and Junctions or Symbolic Links.

Symbolic links are designed to aid in migration and application compatibility with UNIX operating systems. Microsoft has implemented its symbolic links to function just like UNIX links.

A junction (also called a soft link) differs from a hard link in that the storage objects it references are separate directories, and a junction can link directories located on different local volumes on the same computer.

Then we need to consider the junction, which works like a symbolic link to another directory.

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

5 participants