File-name completer marks all files as executable on MS-Windows

Message ID 83egrklxde.fsf@gnu.org
State New, archived
Headers

Commit Message

Eli Zaretskii Dec. 27, 2014, 7:13 p.m. UTC
  I discovered that completing on file names in GDB on MS-Windows marks
every file as executable.  This is because Readline uses 'access' and
X_OK to determine that, which doesn't work on Windows.

Suggested patch is below.

2014-12-27  Eli Zaretskii  <eliz@gnu.org>

	* complete.c (stat_char) [_WIN32]: Don't use 'access' and X_OK on
	Windows, they don't work.  Instead, look at the file-name
	extension to determine whether the file is executable.
  

Comments

Andrew Pinski Dec. 27, 2014, 10 p.m. UTC | #1
On Sat, Dec 27, 2014 at 11:13 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> I discovered that completing on file names in GDB on MS-Windows marks
> every file as executable.  This is because Readline uses 'access' and
> X_OK to determine that, which doesn't work on Windows.
>
> Suggested patch is below.

Has this gone upstream to readline?  Maybe it should go upstream first.

Thanks,
Andrew Pinski

>
> 2014-12-27  Eli Zaretskii  <eliz@gnu.org>
>
>         * complete.c (stat_char) [_WIN32]: Don't use 'access' and X_OK on
>         Windows, they don't work.  Instead, look at the file-name
>         extension to determine whether the file is executable.
>
> --- readline/complete.c~0       2014-06-11 19:34:41.000000000 +0300
> +++ readline/complete.c 2014-12-27 21:06:38.255053100 +0200
> @@ -598,8 +598,21 @@ stat_char (filename)
>  #endif
>    else if (S_ISREG (finfo.st_mode))
>      {
> +#if defined (_WIN32) && !defined (__CYGWIN__)
> +      /* Windows 'access' doesn't support X_OK and on latest Windows
> +        versions even invokes an invalid parameter exception.  */
> +      char *ext = strrchr (filename, '.');
> +
> +      if (ext
> +         && (_rl_stricmp (ext, ".exe") == 0
> +             || _rl_stricmp (ext, ".cmd") == 0
> +             || _rl_stricmp (ext, ".bat") == 0
> +             || _rl_stricmp (ext, ".com") == 0))
> +       character = '*';
> +#else
>        if (access (filename, X_OK) == 0)
>         character = '*';
> +#endif
>      }
>    return (character);
>  }
  
Eli Zaretskii Dec. 28, 2014, 3:38 a.m. UTC | #2
> Date: Sat, 27 Dec 2014 14:00:45 -0800
> From: Andrew Pinski <pinskia@gmail.com>
> Cc: Chet Ramey <chet.ramey@case.edu>, 
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> On Sat, Dec 27, 2014 at 11:13 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > I discovered that completing on file names in GDB on MS-Windows marks
> > every file as executable.  This is because Readline uses 'access' and
> > X_OK to determine that, which doesn't work on Windows.
> >
> > Suggested patch is below.
> 
> Has this gone upstream to readline?  Maybe it should go upstream first.

That's why I CC'ed Chet.
  
Chet Ramey Dec. 29, 2014, 3:38 p.m. UTC | #3
On 12/27/14 2:13 PM, Eli Zaretskii wrote:
> I discovered that completing on file names in GDB on MS-Windows marks
> every file as executable.  This is because Readline uses 'access' and
> X_OK to determine that, which doesn't work on Windows.

Ugh, really?  Thanks for the patch.

Chet
  
Eli Zaretskii Dec. 29, 2014, 4:22 p.m. UTC | #4
> Date: Mon, 29 Dec 2014 10:38:45 -0500
> From: Chet Ramey <chet.ramey@case.edu>
> CC: chet.ramey@case.edu, gdb-patches@sourceware.org
> 
> On 12/27/14 2:13 PM, Eli Zaretskii wrote:
> > I discovered that completing on file names in GDB on MS-Windows marks
> > every file as executable.  This is because Readline uses 'access' and
> > X_OK to determine that, which doesn't work on Windows.
> 
> Ugh, really?  Thanks for the patch.

Thanks for reviewing it.

Joel, Pedro, others: can I now commit this to the GDB repository?
Branch too?
  
Chet Ramey Dec. 29, 2014, 4:46 p.m. UTC | #5
On 12/29/14 11:22 AM, Eli Zaretskii wrote:
>> Date: Mon, 29 Dec 2014 10:38:45 -0500
>> From: Chet Ramey <chet.ramey@case.edu>
>> CC: chet.ramey@case.edu, gdb-patches@sourceware.org
>>
>> On 12/27/14 2:13 PM, Eli Zaretskii wrote:
>>> I discovered that completing on file names in GDB on MS-Windows marks
>>> every file as executable.  This is because Readline uses 'access' and
>>> X_OK to determine that, which doesn't work on Windows.
>>
>> Ugh, really?  Thanks for the patch.
> 
> Thanks for reviewing it.
> 
> Joel, Pedro, others: can I now commit this to the GDB repository?

I am not certain of the exact vintage of the copy of readline in the GDB
repository, but you should probably audit it and see whether or not this
patch should use `fn' instead of `filename'.
  
Eli Zaretskii Dec. 29, 2014, 5:34 p.m. UTC | #6
> Date: Mon, 29 Dec 2014 11:46:48 -0500
> From: Chet Ramey <chet.ramey@case.edu>
> CC: chet.ramey@case.edu, gdb-patches@sourceware.org
> 
> I am not certain of the exact vintage of the copy of readline in the GDB
> repository, but you should probably audit it and see whether or not this
> patch should use `fn' instead of `filename'.

OK, will do.  Thanks.
  
Pedro Alves Dec. 29, 2014, 6:59 p.m. UTC | #7
On 12/29/2014 04:22 PM, Eli Zaretskii wrote:
>> Date: Mon, 29 Dec 2014 10:38:45 -0500
>> From: Chet Ramey <chet.ramey@case.edu>
>> CC: chet.ramey@case.edu, gdb-patches@sourceware.org
>>
>> On 12/27/14 2:13 PM, Eli Zaretskii wrote:
>>> I discovered that completing on file names in GDB on MS-Windows marks
>>> every file as executable.  This is because Readline uses 'access' and
>>> X_OK to determine that, which doesn't work on Windows.
>>
>> Ugh, really?  Thanks for the patch.
> 
> Thanks for reviewing it.
> 
> Joel, Pedro, others: can I now commit this to the GDB repository?
> Branch too?

Fine with me.

Thanks,
Pedro Alves
  
Eli Zaretskii Dec. 30, 2014, 7:30 p.m. UTC | #8
> Date: Mon, 29 Dec 2014 18:59:48 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> On 12/29/2014 04:22 PM, Eli Zaretskii wrote:
> >> Date: Mon, 29 Dec 2014 10:38:45 -0500
> >> From: Chet Ramey <chet.ramey@case.edu>
> >> CC: chet.ramey@case.edu, gdb-patches@sourceware.org
> >>
> >> On 12/27/14 2:13 PM, Eli Zaretskii wrote:
> >>> I discovered that completing on file names in GDB on MS-Windows marks
> >>> every file as executable.  This is because Readline uses 'access' and
> >>> X_OK to determine that, which doesn't work on Windows.
> >>
> >> Ugh, really?  Thanks for the patch.
> > 
> > Thanks for reviewing it.
> > 
> > Joel, Pedro, others: can I now commit this to the GDB repository?
> > Branch too?
> 
> Fine with me.

Thanks, done.
  

Patch

--- readline/complete.c~0	2014-06-11 19:34:41.000000000 +0300
+++ readline/complete.c	2014-12-27 21:06:38.255053100 +0200
@@ -598,8 +598,21 @@  stat_char (filename)
 #endif
   else if (S_ISREG (finfo.st_mode))
     {
+#if defined (_WIN32) && !defined (__CYGWIN__)
+      /* Windows 'access' doesn't support X_OK and on latest Windows
+	 versions even invokes an invalid parameter exception.  */
+      char *ext = strrchr (filename, '.');
+
+      if (ext
+	  && (_rl_stricmp (ext, ".exe") == 0
+	      || _rl_stricmp (ext, ".cmd") == 0
+	      || _rl_stricmp (ext, ".bat") == 0
+	      || _rl_stricmp (ext, ".com") == 0))
+	character = '*';
+#else
       if (access (filename, X_OK) == 0)
 	character = '*';
+#endif
     }
   return (character);
 }