Subject: bug#70914: 29.3; Crashes often on Windows

From: Simen Endsjø <[email-masked]>
Date: Thu, 23 May 2024 20:33:34 +0200

> Got it.  I'm closing this bug, since there doesn't seem to be anything
> left to do here.

Thanks, great job to all of you! Have to say this has been a pleasant support
experience :)

Do you need anything from me? Should I create a patch, or are one of you on it?

On Thu, May 23, 2024 at 6:02 PM Eli Zaretskii <[email-masked]> wrote:
>
> > From: Ihor Radchenko <[email-masked]>
> > Cc: [email-masked], [email-masked], [email-masked], [email-masked]
> > Date: Thu, 23 May 2024 14:23:21 +0000
> >
> > Eli Zaretskii <[email-masked]> writes:
> >
> > >> > My point is that if the URI is "file://", the corresponding :path on
> > >> > Windows is not "//".
> > >>
> > >> Sure. But what is passed to :activate-func is not a file path. It is
> > >> "Org mode link path". They are not the same in this specific case, but
> > >> there is nothing wrong on Org side.
> > >
> > > Then on whose side is something wrong in this case?  Which code calls
> > > file-exists-p with the string "//" in the case you described?
> >
> > https://github.com/doomemacs/doomemacs/blob/master/modules/lang/org/config.el#L500
> >
> > >> >> IMHO, the bug is in file-exists-p:
> > >> >
> > >> > I don't think so.  "//" is an invalid file name on Windows, so the
> > >> > only requirement from file-exists-p in that case is not to crash.
> > >> > Which it does, after the recent fix.
> > >>
> > >> IMHO, "invalid file name on Windows" leads to "trouble determining
> > >> whether the file exists".
> > >
> > > Since the file name "//" is invalid, you get undefined behavior.
> >
> > > Examples:
> > >
> > >   (file-exists-p "//)
> > >     => nil
> >
> > As I tried to explain, I do not see the behaviour as "undefined" when the
> > file name is really invalid. Not for `file-exists-p'. AFAIU,
> > `file-attributes' and `file-directory-p' do not give such promises in
> > their docstrings.
> >
> > > but
> > >
> > >   (file-attributes "//")
> > >     => (t 1 1001 513 (0 0 0 0) (0 0 0 0) (0 0 0 0) 0 "drwxrwxrwx" t 0 0)
> >
> > This is somewhat surprising, but also somewhat expected -
> > `file-exists-p' does not always mean that file does not exist: "This
> > returns nil for a symlink to a nonexistent file." Yet, symlink to a
> > nonexistent file can have attributes. So, I can see how
> > `file-attributes' can return something strange for non-existing files.
> >
> > > and
> > >
> > >   (file-directory-p "//")
> > >     => nil
> >
> > This also follows from the docstring of `file-directory-p':
> >
> >     Return t if FILENAME names an existing directory.
> >
> >     Return nil if FILENAME does not name a directory, or if there
> >     was trouble determining whether FILENAME is a directory.
> >
> > >> AFAIU, this is what happens after the fix anyway, so there is nothing
> > >> more that should be done on Emacs side.
> > >
> > > Generating such invalid file names is wrong, so it should be prevented
> > > if possible.  But I won't argue more if you are still unconvinced.
> >
> > Org does not care about "file" part. There is a link. The link has a
> > path. User can put anything in that path. Org does not generate file
> > names - Org parses Org link syntax and leaves its interpretation up to
> > the more specialized code.
> >
> > Of course, if you see problems with Org mode code that interprets file
> > links specifically and are _also_ designed to handle file links, I will
> > be happy to fix edge cases like you described. The fontification code in
> > Org mode has nothing to do with interpreting the meaning of the links.
>
> Got it.  I'm closing this bug, since there doesn't seem to be anything
> left to do here.