[Bug other/108276] New: libiberty unlink_if_ordinary does not handle Windows nul device correctly

[Bug other/108276] New: libiberty unlink_if_ordinary does not handle Windows nul device correctly

[himalr at proton dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

            Bug ID: 108276
           Summary: libiberty unlink_if_ordinary does not handle Windows
                    nul device correctly
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: himalr at proton dot me
  Target Milestone: ---

Created attachment 54180
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54180&action=edit
Handle Windows nul device correctly in unlink_if_ordinary function

I reported this as a part of another bug to [Sourceware
Bugzilla](https://sourceware.org/bugzilla/show_bug.cgi?id=29947) and was asked
to report this here.

In Windows, 'stat' function (and in turn, S_ISREG) returns true for null device
as a regular file.

Please note that I'm not a C programmer so let me know if there are any issues
with the patch.

Thanks.

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patches should be sent to gcc-patches@ after reading
https://gcc.gnu.org/contribute.html

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #2 from niXman <i.nixman at autistici dot org> ---
@Himal

```
+#ifdef _WIN32
+    if (stricmp (name, "nul") == 0)
+      return 1;
+#else
   struct stat st;

   if (lstat (name, &st) == 0
       && (S_ISREG (st.st_mode) || S_ISLNK (st.st_mode)))
     return unlink (name);
+#endif

```

I don't think the patch is correct because for WIN32 platform `unlink()` will
never be called even for non-"nul" files.

right?

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #3 from niXman <i.nixman at autistici dot org> ---
and I propose to use `strcasecmp()`

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #4 from niXman <i.nixman at autistici dot org> ---
(In reply to niXman from comment #2)

> I don't think the patch is correct because for WIN32 platform `unlink()`
> will never be called even for non-"nul" files.

moreover, according to the man:
https://github.com/MicrosoftDocs/cpp-docs/blob/main/docs/c-runtime-library/reference/unlink-wunlink.md

microsoft's `unlink()/_unlink()` works as just `remove()`.

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[himalr at proton dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #5 from Himal <himalr at proton dot me> ---
(In reply to niXman from comment #4)
> (In reply to niXman from comment #2)
> 
> > I don't think the patch is correct because for WIN32 platform `unlink()`
> > will never be called even for non-"nul" files.
> 
> moreover, according to the man:
> https://github.com/MicrosoftDocs/cpp-docs/blob/main/docs/c-runtime-library/
> reference/unlink-wunlink.md
> 
> microsoft's `unlink()/_unlink()` works as just `remove()`.

Hi, Thanks for the feedback. I noticed this while looking for
[this](https://sourceware.org/bugzilla/show_bug.cgi?id=29947) bug. 

In MSYS/MinGW, It calls the `unlink` function even for 'nul', but it wasn't
throwing any errors.

I decided to submit this patch because it already has some code checking for
special devices before calling `unlink`. If it's not required then I guess we
can close this.

Thanks.

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[i.nixman at autistici dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #6 from niXman <i.nixman at autistici dot org> ---
I think you don't understand me.

with your patch after preprocessing the `unlink_if_ordinary()` will look like:
```
int
unlink_if_ordinary (const char *name)
{
  if (stricmp (name, "nul") == 0)
    return 1;

  return 1;
}

```

don't you think it's strange?

[Bug other/108276] libiberty unlink_if_ordinary does not handle Windows nul device correctly

[himalr at proton dot me]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108276

--- Comment #7 from Himal <himalr at proton dot me> ---
(In reply to niXman from comment #6)
> I think you don't understand me.
> 
> with your patch after preprocessing the `unlink_if_ordinary()` will look
> like:
> ```
> int
> unlink_if_ordinary (const char *name)
> {
>   if (stricmp (name, "nul") == 0)
>     return 1;
> 
>   return 1;
> }
> 
> ```
> 
> don't you think it's strange?

Ah, yes. I noticed that after submitting the patch. I have already posted an
updated version
[here](https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609487.html)