Re: test -x should use faccessat, not stat

2010-04-02 Thread Eric Blake
According to Herbert Xu on 4/2/2010 8:03 AM:
> After much deliberation (alright, I've simply been busy elsewhere :)
> I've committed this patch.
> 
> commit 1d68712ba2e439f36874c4ed1e3d9ffec177a06c
> Note that faccessat doesn't handle ACLs when euid != uid, as
> this case is currently implemented by glibc instead of the kernel,
> using code similar to the existing dash test.

That faccessat bug is only true for current Linux kernels.  Cygwin
faccessat does the correct thing, even when euid != uid.

Thanks for applying this.

-- 
Don't work too hard, make some time for fun as well!

Eric Blake e...@byu.net
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: (fwd) dash fix for job control off warning

2010-04-02 Thread maximilian attems
On Fri, Apr 02, 2010 at 10:42:20PM +0800, Herbert Xu wrote:
> On Fri, Apr 02, 2010 at 04:32:20PM +0200, maximilian attems wrote:
> >
> > Pulled this; there seems to be a problem with the new version of dash
> > with job control off.  I can't tell if it is just a warning or is a
> > manifest bug.
> > 
> > The solution is simple:
> > 
> > --- a/usr/dash/jobs.h
> > +++ b/usr/dash/jobs.h
> > @@ -105,5 +105,5 @@ int waitforjob(struct job *);
> >  int stoppedjobs(void);
> > 
> >  #if ! JOBS
> > -#define setjobctl(on)  /* do nothing */
> > +#define setjobctl(on) ((void)(on)) /* do nothing */
> >  #endif
> > 
> > ... to keep the code syntactically valid even when setjobctl() is used
> > as the body of an if statement.
> 
> So when exactly is this needed? Can you give an example?

usr/dash/trap.c: In function `exitshell':
usr/dash/trap.c:376: warning: suggest braces around empty body in an `if' 
statement

exitshell() has:

/*
 * Disable job control so that whoever had the foreground before we
 * started can get it back.
 */
if (likely(!setjmp(loc.loc)))
setjobctl(0);


aboves patch fixes this gcc warning for me, but I still see:
  KLIBCCC usr/dash/trap.o
usr/dash/trap.c: In function ‘exitshell’:
usr/dash/trap.c:352: warning: variable ‘status’ might be clobbered by 
‘longjmp’ or ‘vfork’

--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: (fwd) dash fix for job control off warning

2010-04-02 Thread Herbert Xu
On Fri, Apr 02, 2010 at 04:32:20PM +0200, maximilian attems wrote:
>
> Pulled this; there seems to be a problem with the new version of dash
> with job control off.  I can't tell if it is just a warning or is a
> manifest bug.
> 
> The solution is simple:
> 
> --- a/usr/dash/jobs.h
> +++ b/usr/dash/jobs.h
> @@ -105,5 +105,5 @@ int waitforjob(struct job *);
>  int stoppedjobs(void);
> 
>  #if ! JOBS
> -#define setjobctl(on)  /* do nothing */
> +#define setjobctl(on) ((void)(on)) /* do nothing */
>  #endif
> 
> ... to keep the code syntactically valid even when setjobctl() is used
> as the body of an if statement.

So when exactly is this needed? Can you give an example?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


(fwd) dash fix for job control off warning

2010-04-02 Thread maximilian attems
- Forwarded message from "H. Peter Anvin"  -

Date: Mon, 29 Mar 2010 15:07:01 -0700
From: "H. Peter Anvin" 
To: maximilian attems 
Cc: Colin Watson , kl...@zytor.com,
Herbert Xu ,
Nobuhiro Iwamatsu 
Subject: Re: [klibc] [git pull v3] dash, sh4, ipconfig, dprintf, fstype,
README's

On 03/27/2010 05:58 PM, maximilian attems wrote:
> hello hpa!
> 
> added on top of queue ext4 fix, that Ubuntu is carrying from cjwatson.
> Got missed out in previous pull requests. Btrfs recognition in fstype.
> 
> the patch queue contains sync with latest dash, sh4 fix by Debian porters,
> dprintf usage in ipconfig, kinit and nfsmount instead of buggy DEBUG
> macro. ipconfig memcpy usage to avoid strict aliasing warnings.
> Some interesting README's got renamed for easier packaging.
> Bonus is sparc32 socket test.
> 

Pulled this; there seems to be a problem with the new version of dash
with job control off.  I can't tell if it is just a warning or is a
manifest bug.

The solution is simple:

--- a/usr/dash/jobs.h
+++ b/usr/dash/jobs.h
@@ -105,5 +105,5 @@ int waitforjob(struct job *);
 int stoppedjobs(void);

 #if ! JOBS
-#define setjobctl(on)  /* do nothing */
+#define setjobctl(on) ((void)(on)) /* do nothing */
 #endif

... to keep the code syntactically valid even when setjobctl() is used
as the body of an if statement.

-hpa

- End forwarded message -
-- 
maks
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] [BUILTIN] respect eventual TMPDIR set

2010-04-02 Thread maximilian attems
On Fri, 02 Apr 2010, Herbert Xu wrote:

> On Mon, Mar 22, 2010 at 12:23:38AM +0100, maximilian attems wrote:
> > From: Herbert Xu 
> > 
> > while merging upstream dash in klibc,
> > noticed that klibc dash had grown that useful feature.
> > 
> > Signed-off-by: maximilian attems 
> 
> Patch applied.  BTW, please cc dash@vger.kernel.org on all patch
> submissions.

thanks for merge, review and push. :)

what about, job control off:
http://www.zytor.com/pipermail/klibc/2010-March/002590.html

I'll reforward the patch, seems it didn't reach the dash mailinglist.

--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: test -x should use faccessat, not stat

2010-04-02 Thread Herbert Xu
On Tue, Feb 16, 2010 at 08:04:13PM +0800, Herbert Xu wrote:
> 
> First of all I completely agree with you that the current dash
> behaviour is busted.  As to how we should fix it, let me think
> about this a little more before making a decision.

After much deliberation (alright, I've simply been busy elsewhere :)
I've committed this patch.

commit 1d68712ba2e439f36874c4ed1e3d9ffec177a06c
Author: Herbert Xu 
Date:   Fri Apr 2 22:02:22 2010 +0800

[BUILTIN] Use faccessat if available

Eric Blake suggested that we should use faccessat so that ACLs
and other corner cases are handled correctly.  This patch does
exactly that.

Note that faccessat doesn't handle ACLs when euid != uid, as
this case is currently implemented by glibc instead of the kernel,
using code similar to the existing dash test.

Signed-off-by: Herbert Xu 

diff --git a/ChangeLog b/ChangeLog
index 7af5070..9f63819 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2010-04-02  Herbert Xu 
 
+   * Use faccessat if available.
+
+2010-04-02  Herbert Xu 
+
* Make trap signal name/number errors non-fatal.
* Release 0.5.6.
 
diff --git a/configure.ac b/configure.ac
index df6e099..c943725 100644
--- a/configure.ac
+++ b/configure.ac
@@ -46,7 +46,8 @@ dnl Checks for header files.
 AC_CHECK_HEADERS(alloca.h)
 
 dnl Checks for library functions.
-AC_CHECK_FUNCS(bsearch getpwnam getrlimit imaxdiv isalpha killpg mempcpy \
+AC_CHECK_FUNCS(bsearch faccessat getpwnam getrlimit imaxdiv isalpha killpg \
+  mempcpy \
   sigsetmask stpcpy strchrnul strsignal strtod strtoimax \
   strtoumax sysconf)
 
diff --git a/src/bltin/test.c b/src/bltin/test.c
index 8e7077a..7888f38 100644
--- a/src/bltin/test.c
+++ b/src/bltin/test.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -147,8 +148,12 @@ static int isoperand(char **);
 static int newerf(const char *, const char *);
 static int olderf(const char *, const char *);
 static int equalf(const char *, const char *);
+#ifdef HAVE_FACCESSAT
+static int test_file_access(const char *, int);
+#else
 static int test_st_mode(const struct stat64 *, int);
 static int bash_group_member(gid_t);
+#endif
 
 static inline intmax_t getn(const char *s)
 {
@@ -295,6 +300,14 @@ primary(enum token n)
return strlen(*t_wp) != 0;
case FILTT:
return isatty(getn(*t_wp));
+#ifdef HAVE_FACCESSAT
+   case FILRD:
+   return test_file_access(*t_wp, R_OK);
+   case FILWR:
+   return test_file_access(*t_wp, W_OK);
+   case FILEX:
+   return test_file_access(*t_wp, X_OK);
+#endif
default:
return filstat(*t_wp, n);
}
@@ -364,12 +377,14 @@ filstat(char *nm, enum token mode)
return 0;
 
switch (mode) {
+#ifndef HAVE_FACCESSAT
case FILRD:
return test_st_mode(&s, R_OK);
case FILWR:
return test_st_mode(&s, W_OK);
case FILEX:
return test_st_mode(&s, X_OK);
+#endif
case FILEXIST:
return 1;
case FILREG:
@@ -469,6 +484,12 @@ equalf (const char *f1, const char *f2)
b1.st_ino == b2.st_ino);
 }
 
+#ifdef HAVE_FACCESSAT
+static int test_file_access(const char *path, int mode)
+{
+   return !faccessat(AT_FDCWD, path, mode, AT_EACCESS);
+}
+#else  /* HAVE_FACCESSAT */
 /*
  * Similar to what access(2) does, but uses the effective uid and gid.
  * Doesn't make the mistake of telling root that any file is executable.
@@ -519,3 +540,4 @@ bash_group_member(gid_t gid)
 
return (0);
 }
+#endif /* HAVE_FACCESSAT */

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html