This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] cygcheck.cc update for cygpath()


Hi Brian,

Thanks for your patch.  I have a few nits, sorry.

On Mar  8 20:13, Brian Dessent wrote:
> Index: cygcheck.cc
> ===================================================================
> RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
> retrieving revision 1.97
> diff -u -p -r1.97 cygcheck.cc
> --- cygcheck.cc	13 Jan 2008 13:41:45 -0000	1.97
> +++ cygcheck.cc	9 Mar 2008 03:52:07 -0000
> @@ -807,6 +807,31 @@ ls (char *f)
>      display_error ("ls: CloseHandle()");
>  }
>  
> +/* If s is non-NULL, save the CWD in a static buffer and set the CWD
> +   to the dirname part of s.  If s is NULL, restore the CWD last
> +   saved.  */
> +static
> +void save_cwd_helper (const char *s)
> +{
> +  static char cwdbuf[MAX_PATH + 1];
> +  char dirnamebuf[MAX_PATH + 1];
> +
> +  if (s)
> +    {
> +      GetCurrentDirectory (sizeof (cwdbuf), cwdbuf);
> +
> +      /* Remove the filename part from s.  */
> +      strncpy (dirnamebuf, s, MAX_PATH);
> +      dirnamebuf[MAX_PATH] = '\0';   // just in case strlen(s) > MAX_PATH
> +      char *lastsep = strrchr (dirnamebuf, '\\');
> +      if (lastsep)
> +        lastsep[1] = '\0';
> +      SetCurrentDirectory (dirnamebuf);
> +    }
> +  else
> +    SetCurrentDirectory (cwdbuf);
> +}

Given that Cygwin changes to support long path names, I don't really
like to see new code still using MAX_PATH and Win32 Ansi functions
in the utils dir.  I know that the Win32 cwd is always restricted to
259 chars.  However, there *is* a way to recognize the current working
directory of the parent Cygwin application.

Bash as well as tcsh, as well as zsh (and probbaly pdksh, too) create an
environment variable $PWD.  Maybe Cygwin should create $PWD for native
apps if it's not already available through the parent shell.  I'd
suggest that the Cygwin utils first try to fetch $PWD from the
environment and use that as cwd.  Only if that fails, use
GetCurrentDirectory.

Never use SetCurrentDirectory, rather convert the path to an absolute
path, prepend \\?\ and call the Win32 unicode functions (CreateFileW,
etc).

>  // Find a real application on the path (possibly following symlinks)
>  static const char *
>  find_app_on_path (const char *app, bool showall = false)
> @@ -822,25 +847,27 @@ find_app_on_path (const char *app, bool 
>    if (is_symlink (fh))
>      {
>        static char tmp[4000] = "";

SYMLINK_MAX is PATH_MAX - 1 == 4095.

I'm wondering if you would like to tweak the readlink functions, too.
Cygwin shortcuts can now have the path name appended to the actual
shortcut data.  This hack was necessary to support pathnames longer than
2000 chars.  See the comment and code in cygwin/path.cc, line 3139ff.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]