Linux-Dash Archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Edward Lam <edward@sidefx.com>, dash@vger.kernel.org
Subject: Re: [PATCH] Support DOS paths in dash
Date: Tue, 30 Sep 2014 16:02:42 -0600	[thread overview]
Message-ID: <542B2882.9030400@redhat.com> (raw)
In-Reply-To: <51545CD9.4050607@sidefx.com>

[-- Attachment #1: Type: text/plain, Size: 2637 bytes --]

[I noticed an old thread in my inbox while packaging dash 0.5.8 for Cygwin]

On 03/28/2013 09:08 AM, Edward Lam wrote [to the cygwin list]:

>> The problem is that dash tries to convert c:/windows to an absolute
>> path, since it doesn't start with /.  I suppose I could teach dash to
>> recognize [letter]:/ as absolute paths, although that makes dash larger,
>> and puts a burden on me (since I can guarantee upstream dash won't
>> accept such a patch).
>>
>>> I just don't care enough for DOS paths so I won't fix.
>>
>> Me neither.  And since you can use /cygdrive/c, not c:/, I won't bother
>> to fix it.
>>

> Hi Folks,
>
> I finally got down to looking at how to fix this in dash and came up
> with the attached patch (against dash-0.5.7). It's simple enough and so
> cd now works.
>
> Please consider this for Cygwin.
>

I'm not interested in burdening the cygwin build of dash with a one-off
patch, so I'd like to gauge the upstream thoughts - is it worth
including platform-specific patches like this (no penalty to build size
of non-cygwin platforms, and on cygwin, it allows 'cd c:/' to behave as
shorthand for 'cd /cygdrive/c/')?  If the patch lands in dash.git, then
I'll rebuild the cygwin port of dash to include a backport (rather than
waiting for 0.5.9 to be released).  If there is no interest, I'd rather
just drop the patch.  The cygwin community already states that
/cygdrive/c notation is the official way to access drive letters, and
that if 'c:/' works it is nice, but it is not a design goal to always
have it work.

> --- src/cd.c	2011-03-15 03:18:06.000000000 -0400
> +++ src/cd.new.c	2013-03-28 11:03:32.649576500 -0400
> @@ -38,6 +38,9 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <limits.h>
> +#ifdef __CYGWIN__
> +#include <sys/cygwin.h>
> +#endif
>  
>  /*
>   * The cd and pwd commands.
> @@ -194,6 +197,11 @@
>  	char *cdcomppath;
>  	const char *lim;
>  
> +#ifdef __CYGWIN__
> +        char pathbuf[PATH_MAX + 1];
> +        cygwin_conv_to_full_posix_path (dir, pathbuf);

By the way, cygwin_conv_to_full_posix_path() is deprecated (it suffers
from possible buffer overflow); these days, it's preferred to use:

cygwin_conv_path (CCP_WIN_A_TO_POSIX | CCP_RELATIVE, string, pathbuf,
                  sizeof(pathbuf))

So, if there is interest in this patch upstream, I can respin it.

> +	 dir = pathbuf;
> +#endif
>  	cdcomppath = sstrdup(dir);
>  	STARTSTACKSTR(new);
>  	if (*dir != '/') {
> 
> 


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

       reply	other threads:[~2014-09-30 22:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <announce.4A5C7A2B.4020700@byu.net>
     [not found] ` <4B5860C9.50100@sidefx.com>
     [not found]   ` <4B591221.10501@byu.net>
     [not found]     ` <4B91105B.7050205@sidefx.com>
     [not found]       ` <20100305152637.GK7980@calimero.vinschen.de>
     [not found]         ` <4B913788.40301@sidefx.com>
     [not found]           ` <4B913AF2.3070102@redhat.com>
     [not found]             ` <20100305172048.GN7980@calimero.vinschen.de>
     [not found]               ` <4EDA9E99.9030307@redhat.com>
     [not found]                 ` <51545CD9.4050607@sidefx.com>
2014-09-30 22:02                   ` Eric Blake [this message]
2014-10-02  0:41                     ` [PATCH] Support DOS paths in dash Herbert Xu
2014-10-02  3:18                       ` Edward Lam
2014-10-04  4:07                         ` Herbert Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542B2882.9030400@redhat.com \
    --to=eblake@redhat.com \
    --cc=dash@vger.kernel.org \
    --cc=edward@sidefx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).