Re: 'set' leaks garbage from environment

2014-09-30 Thread Eric Blake
On 09/30/2014 09:14 AM, Olof Johansson wrote:
 On 2014-09-30 09:01 -0600, Eric Blake wrote:
 $ dash -c 'unset a|b
 $ echo $?
 0
 
 Works for me (tested on both Debian package versions 0.5.7-3 (wheezy)
 and 0.5.7-4 (unstable)):

Serves me right from testing on multiple machines :(
I mixed up my test results.

Fedora 20 using dash 0.5.7 works:

$ dash -c 'unset a|b'
dash: 1: unset: a|b: bad variable name
$ rpm -q dash
dash-0.5.7-8.fc20.x86_64

But RHEL 6 fails:

$ dash -c 'unset a|b'
$ rpm -q dash
dash-0.5.5.1-4.el6.x86_64

so this is at least one bug that has already been fixed upstream.

 $ env 'a|b=' dash -c 'set | grep a.b'
 a|b=''
 
 This I can reproduce though.

Meanwhile, I just tested the latest dash.git (commit f21016a12) and this
behavior is no longer present:

$ env 'a|b=' ./src/dash -c 'set | grep a.b'

so it has also been fixed in the meantime.  Sorry for not doing my
homework; nothing to fix here...

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



signature.asc
Description: OpenPGP digital signature


Re: [PATCH] Support DOS paths in dash

2014-09-30 Thread Eric Blake
[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.0 -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



signature.asc
Description: OpenPGP digital signature