stat(x) & AT_NO_AUTOMOUNT
Hi, while debugging failed test suites in gdk-pixbuf, I noticed the following: - newer glib versions can use statx - glib provides a statx emulation in case statx is not provided natively, falling back to the usual stat (with less information available) - the statx emulation allows few AT_* flags, namely: AT_EMPTY_PATH | AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW - AT_NO_AUTOMOUNT is not handled in the hurd bits (see __hurd_at_flags and __file_name_lookup_at) - the glib wrappers around statx always pass AT_NO_AUTOMOUNT for non-fd queries (i.e. when using a path name and not an fd) - a number of g_file_* APIs are broken, as they fail with EINVAL Also, this can be triggered when using fstatat() with AT_NO_AUTOMOUNT among the flags. Can AT_NO_AUTOMOUNT be (easily) implemented in the hurd lookup bits? Alternatively, should AT_NO_AUTOMOUNT be excluded in at/fd lookups? Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: stat(x) & AT_NO_AUTOMOUNT
In data sabato 28 novembre 2020 20:02:52 CET, Samuel Thibault ha scritto: > Hello Pino, > > Pino Toscano, le sam. 28 nov. 2020 19:45:12 +0100, a ecrit: > > Can AT_NO_AUTOMOUNT be (easily) implemented in the hurd lookup bits? > > I would say that it simply maps to O_NOTRANS? I thought about that; then I saw in libdiskfs/dir-lookup.c & libnetfs/dir-lookup.c that O_NOTRANS disables the symlink resolution, i.e. the equivalent of O_NOLINK, which means AT_SYMLINK_NOFOLLOW in at flags. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
getpeername() bug with non-connected sockets
Hi,
looking at the Python test suite output, I found an actual bug related
to getpeername(). When called on non-connected sockets, it shall return
ENOTCONN, but instead it returns -ENOTCONN. The attached testcase shows
the issue:
$ ./getpeername
, -1, -1073741881 vs 1073741881
while it should be:
, -1, 1073741881 vs 1073741881
Investigating a bit in the sources of hurd/pfinet, socket-
ops.c:S_socket_peername() just return straight the return value of
misc.c:make_sockaddr_port().
make_sockaddr_port() calls the getname function of the sock-ops of the
current sock struct, which can be one of
- linux-src/net/ipv6/af_inet6.c:inet6_getname()
- linux-src/net/ipv4/af_inet.c:inet_getname()
- linux-src/net/core/sock.c:sock_no_getname()
all of them return negative errno values, which make_sockaddr_port()
return straight those as well.
Not totally sure about where to fix, i.e. a "return -err" in
make_sockaddr_port() after the getname call, or in all the callers of
make_sockaddr_port().
--
Pino Toscano
#include
#include
#include
#include
#include
#include
int main()
{
int s, e;
struct sockaddr_in ss;
socklen_t len;
s = socket(AF_INET, SOCK_STREAM, 0);
len = sizeof(ss);
e = getpeername(s, (struct sockaddr *)&ss, &len);
printf("%d, %d, %d vs %d\n", s, e, errno, ENOTCONN);
close(s);
return 0;
}
signature.asc
Description: This is a digitally signed message part.
Re: getpeername() bug with non-connected sockets
Alle lunedì 11 ottobre 2010, Samuel Thibault ha scritto: > Pino Toscano, le Sun 10 Oct 2010 23:46:10 +0200, a écrit : > > make_sockaddr_port() calls the getname function of the sock-ops of > > the current sock struct, which can be one of > > - linux-src/net/ipv6/af_inet6.c:inet6_getname() > > - linux-src/net/ipv4/af_inet.c:inet_getname() > > - linux-src/net/core/sock.c:sock_no_getname() > > all of them return negative errno values, which > > make_sockaddr_port() return straight those as well. > > Not totally sure about where to fix, i.e. a "return -err" in > > make_sockaddr_port() after the getname call, or in all the callers > > of make_sockaddr_port(). > > Hurd functions don't usually return -errnos, only Linux-inherited > ones do. I see; attached the commit with the fix. -- Pino Toscano From 7aa424fc1cdfeb487a1bfc2e4dfeaac61df51185 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Mon, 11 Oct 2010 17:07:53 +0200 Subject: [PATCH] Fix return value on `getname' errors. This fixes the errno return value for getpeername() calls on not connected sockets. * pfinet/misc.c (make_sockaddr_port): The return value of the `getname' call comes from the Linux code, so it is a negative value, in case of error. Then make it a positive number to get a valid errno value. --- pfinet/misc.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/pfinet/misc.c b/pfinet/misc.c index 56611da..08b19e9 100644 --- a/pfinet/misc.c +++ b/pfinet/misc.c @@ -42,7 +42,7 @@ make_sockaddr_port (struct socket *sock, err = (*sock->ops->getname) (sock, &buf.sa, &buflen, peer); if (err) -return err; +return -err; err = ports_create_port (addrport_class, pfinet_bucket, (offsetof (struct sock_addr, address) -- 1.7.1 signature.asc Description: This is a digitally signed message part.
Re: ED error code
Hi, Alle domenica 31 ottobre 2010, Manuel Menal ha scritto: > The Hurd port of the GNU C Library defines an error code macro called > `ED', which the glibc manual describes as "The experienced user will > know what is wrong.". This error code macro doesn't seem to be used > anywhere. To be precise, a macro and a value of the __error_t_codes enum (which makes it impossible even "#undef ED" to avoid the ED define). > Although having a reserved, generic error code macro might be > useful(?), `ED' is a very generic identifier. For example, clang > uses it in many parts of its code, which makes the build fail on > GNU/Hurd. I think it's likely that we'll meet this problem more than > once. Yes, this hit few package compilations on the Debian buildds. So far I collected: - boost >= 1.40: - boost/math/special_functions/ellint_rd.hpp: local variable `ED' - gentle: - enums.h: enum value `ED' - gpsim: - src/dspic/dspic-instructions.h: class `ED' - httrack: - src/htslib.c: `ED' used as hexadecimal number (the 0x prefix is prepended using a macro) Among the above, I fixed boost renaming the variable to ED_ (with a patch in the Debian packaging), otherwise many important packages couldn't have been compiled. > Couldn't it be renamed to something less generic? Or just removed, IMHO. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
st_dev in struct stat
Hi,
just looking at a recent build failure in Debian, "rdfind".
There's some code like:
FileInfo.hh:
...
struct Fileinfostat {
filesizetype st_size;//size
unsigned long st_ino;//inode
unsigned long st_dev;//device
...
};
FileInfo.cc:
#include "FileInfo.hh"
...
#include
...
struct stat info;
...
instance_of_struct_Fileinfostat.st_dev = info.st_dev;
Such code (a simplified version of which is attached) fails to compile
on Hurd, because in bits/stat.h there is:
struct stat
{
...
__fsid_t st_fsid; /* File system ID. */
#define st_dev st_fsid
Is this allowed by POSIX (most probly I'm missing the right part(s) of
it)?
If not, would a "fix" for the above be swapping member and define, like:
__fsid_t st_dev; /* File system ID. */
#define st_fsid st_dev
?
Thanks,
--
Pino Toscano
struct MyTest
{
int st_dev;
};
#include
int main()
{
struct MyTest t;
t.st_dev = 0;
return 0;
}
signature.asc
Description: This is a digitally signed message part.
Re: [Fwd: Questions on isc-dhcp]
Alle giovedì 24 febbraio 2011, Samuel Thibault ha scritto: > Svante Signell, le Thu 24 Feb 2011 12:42:13 +0100, a écrit : > > I also still have the following problem with dbus during boot after > > installing the new libc and pfinet: > > Starting message bus: dbus > > Failed to set socket option "/var/run/dbus/system_bus_socket" > > Please investigate. There is a lot of such small issues here and > there that are "known", but without infinite amount of free time we > can't take the some to deal them all. It looks like it is trying to setsockopt(SO_REUSEADDR) on an AF_UNIX socket, which makes pflocal return EOPNOTSUPP. By default, the warning message for this setsockopt(9 is not fatail though. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [Fwd: Questions on isc-dhcp]
Alle mercoledì 2 marzo 2011, Svante Signell ha scritto:
> Package: isc-dhcp-client
> Architecture: any
> Depends: debianutils (>= 2.8.2), isc-dhcp-common (=
> ${binary:Version}),
>
> -iproute [linux-any], ${shlibs:Depends}, ${misc:Depends}
> (I'm not sure about the syntax here)
> +iproute [linux-any]| inetutils-tools [hurd] , ${shlibs:Depends},
>
> ${misc:Depends}
iproute [linux-any] | inetutils-tools [hurd-any]
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH 3/4] Add a monotonic time variable
* kern/mach_clock.c (monotonic, mmonotonic): New variables.
(clock_interrupt): Call `time_value_add_usec' also on `monotonic', and
`update_mapped_time' also for `mmonotonic'.
(mapable_time_init): Call `mapable_time_init_time' also `monotonic' and
`mmonotonic'.
---
kern/mach_clock.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/kern/mach_clock.c b/kern/mach_clock.c
index 459a290..1e3b88e 100644
--- a/kern/mach_clock.c
+++ b/kern/mach_clock.c
@@ -71,6 +71,7 @@ void mapable_time_init_time();/* forward */
inthz = HZ;/* number of ticks per second */
inttick = (100 / HZ); /* number of usec per tick */
time_value_t time = { 0, 0 };/* time since bootup (uncorrected) */
+time_value_t monotonic = { 0, 0 }; /* time since bootup (uncorrected) */
unsigned long elapsed_ticks = 0; /* ticks elapsed since bootup */
inttimedelta = 0;
@@ -96,6 +97,7 @@ int bigadj = 100; /* adjust 10*tickadj if
adjustment
*/
mapped_time_value_t *mtime = 0;
+mapped_time_value_t *mmonotonic = 0;
#define update_mapped_time(time, mtime)\
MACRO_BEGIN\
@@ -221,6 +223,7 @@ void clock_interrupt(usec, usermode, basepri)
*/
if (timedelta == 0) {
time_value_add_usec(&time, usec);
+ time_value_add_usec(&monotonic, usec);
}
else {
register intdelta;
@@ -234,8 +237,10 @@ void clock_interrupt(usec, usermode, basepri)
timedelta -= tickdelta;
}
time_value_add_usec(&time, delta);
+ time_value_add_usec(&monotonic, delta);
}
update_mapped_time(&time, mtime);
+ update_mapped_time(&monotonic, mmonotonic);
/*
* Schedule soft-interupt for timeout if needed
@@ -497,6 +502,7 @@ host_adjust_time(host, new_adjustment, old_adjustment)
void mapable_time_init()
{
mapable_time_init_time(&time, &mtime, "mapable_time_init_mtime");
+ mapable_time_init_time(&monotonic, &mmonotonic,
"mapable_time_init_mmonotonic");
}
void mapable_time_init_time(time_v, mapped_time_v, what)
--
1.7.5.4
[PATCH 0/4] Monotonic clock device in GNU Mach
Hi, after Samuel's hint about mapped time, I've been able to add a new clock in GNU Mach, only updated on clock interrupts. Also, this clock is exposed like as device like the "time" one. Pino Toscano (4): Make update_mapped_time() take also the mapped time variable Make `mapable_time_init' parametric Add a monotonic time variable Add a monotonic time device i386/i386at/conf.c |8 i386/i386at/model_dep.c | 15 +++ kern/mach_clock.c | 37 ++--- 3 files changed, 49 insertions(+), 11 deletions(-) -- 1.7.5.4
[PATCH 1/4] Make update_mapped_time() take also the mapped time variable
* kern/mach_clock.c (update_mapped_time): Add a second parameter for the
mapped time variable, instead of harcoding it to `mtime'.
---
kern/mach_clock.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/kern/mach_clock.c b/kern/mach_clock.c
index 4ba7c08..af221f5 100644
--- a/kern/mach_clock.c
+++ b/kern/mach_clock.c
@@ -96,14 +96,14 @@ int bigadj = 100; /* adjust 10*tickadj if
adjustment
mapped_time_value_t *mtime = 0;
-#define update_mapped_time(time) \
+#define update_mapped_time(time, mtime)\
MACRO_BEGIN\
- if (mtime != 0) { \
- mtime->check_seconds = (time)->seconds; \
+ if ((mtime) != 0) { \
+ (mtime)->check_seconds = (time)->seconds; \
asm volatile("":::"memory");\
- mtime->microseconds = (time)->microseconds; \
+ (mtime)->microseconds = (time)->microseconds; \
asm volatile("":::"memory");\
- mtime->seconds = (time)->seconds; \
+ (mtime)->seconds = (time)->seconds; \
} \
MACRO_END
@@ -234,7 +234,7 @@ void clock_interrupt(usec, usermode, basepri)
}
time_value_add_usec(&time, delta);
}
- update_mapped_time(&time);
+ update_mapped_time(&time, mtime);
/*
* Schedule soft-interupt for timeout if needed
@@ -428,7 +428,7 @@ host_set_time(host, new_time)
s = splhigh();
time = new_time;
- update_mapped_time(&time);
+ update_mapped_time(&time, mtime);
resettodr();
splx(s);
@@ -499,7 +499,7 @@ void mapable_time_init()
!= KERN_SUCCESS)
panic("mapable_time_init");
memset(mtime, 0, PAGE_SIZE);
- update_mapped_time(&time);
+ update_mapped_time(&time, mtime);
}
int timeopen()
--
1.7.5.4
[PATCH 2/4] Make `mapable_time_init' parametric
* kern/mach_clock.c (mapable_time_init_time): New function doing the job
previously done by `mapable_time_init', taking the time variables and the
string for the job.
(mapable_time_init): Call `mapable_time_init_time' for `time' and `mtime'.
---
kern/mach_clock.c | 17 +
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/kern/mach_clock.c b/kern/mach_clock.c
index af221f5..459a290 100644
--- a/kern/mach_clock.c
+++ b/kern/mach_clock.c
@@ -66,6 +66,7 @@
#endif
void softclock(); /* forward */
+void mapable_time_init_time(); /* forward */
inthz = HZ;/* number of ticks per second */
inttick = (100 / HZ); /* number of usec per tick */
@@ -495,11 +496,19 @@ host_adjust_time(host, new_adjustment, old_adjustment)
void mapable_time_init()
{
- if (kmem_alloc_wired(kernel_map, (vm_offset_t *) &mtime, PAGE_SIZE)
+ mapable_time_init_time(&time, &mtime, "mapable_time_init_mtime");
+}
+
+void mapable_time_init_time(time_v, mapped_time_v, what)
+ time_value_t*time_v;
+ mapped_time_value_t **mapped_time_v;
+ const char *what;
+{
+ if (kmem_alloc_wired(kernel_map, (vm_offset_t *) mapped_time_v,
PAGE_SIZE)
!= KERN_SUCCESS)
- panic("mapable_time_init");
- memset(mtime, 0, PAGE_SIZE);
- update_mapped_time(&time, mtime);
+ panic(what);
+ memset(*mapped_time_v, 0, PAGE_SIZE);
+ update_mapped_time(time_v, *mapped_time_v);
}
int timeopen()
--
1.7.5.4
[PATCH 4/4] Add a monotonic time device
* i386/i386at/conf.c (dev_name_list): Add a new `monotonic' device.
* i386/i386at/model_dep.c (monotonicmmap): New function to map `mmonotonic'.
---
i386/i386at/conf.c |8
i386/i386at/model_dep.c | 15 +++
2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/i386/i386at/conf.c b/i386/i386at/conf.c
index 83c8dbf..e5aff33 100644
--- a/i386/i386at/conf.c
+++ b/i386/i386at/conf.c
@@ -74,6 +74,9 @@ extern inthypcngetstat(), hypcnsetstat(),
hypcnportdeath();
#define hypcnname "hyp"
#endif /* MACH_HYP */
+extern vm_offset_t monotonicmmap();
+#definemonotonicname "monotonic"
+
/*
* List of devices - console must be at slot 0
*/
@@ -149,6 +152,11 @@ struct dev_ops dev_name_list[] =
nodev },
#endif /* MACH_HYP */
+ { monotonicname,timeopen, timeclose, nulldev,
+ nulldev, nulldev,nulldev,monotonicmmap,
+ nodev,nulldev,nulldev,0,
+ nodev },
+
};
intdev_name_count = sizeof(dev_name_list)/sizeof(dev_name_list[0]);
diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
index 04b8228..e929d22 100644
--- a/i386/i386at/model_dep.c
+++ b/i386/i386at/model_dep.c
@@ -586,6 +586,21 @@ timemmap(dev,off,prot)
return (i386_btop(pmap_extract(pmap_kernel(), (vm_offset_t) mtime)));
}
+int
+monotonicmmap(dev,off,prot)
+ vm_prot_t prot;
+{
+ extern time_value_t *mmonotonic;
+
+#ifdef lint
+ dev++; off++;
+#endif /* lint */
+
+ if (prot & VM_PROT_WRITE) return (-1);
+
+ return (i386_btop(pmap_extract(pmap_kernel(), (vm_offset_t)
mmonotonic)));
+}
+
void
startrtclock(void)
{
--
1.7.5.4
[PATCH] add missing format strings
Hi,
Debian recently started to set slightly more stricter C(XX)FLAGS for
packages, and among those there are:
-Wformat -Wformat-security -Werror=format-security
There are few calls to error(), printk() and problem() which have no
format string, causing them to fail the build of hurd.
Attached there is a git-format'ed patch to fix all (hopefully) of those
cases.
--
Pino Toscano
From 2ee8a64be2d79eb006eebbac95c859f1fc7676f0 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Tue, 18 Oct 2011 23:46:36 +0200
Subject: [PATCH] Add missing format strings for error, printk, problem
Some calls to `error', `printk', and `problem' lacked a format
string, leading to build failure when compiling with stricter CFLAGS.
* nfs/mount.c (mount_root): Add format string for `error' calls which
lacked it.
* pfinet/main.c (pfinet_bind): Likewise.
* term/main.c (main): Likewise.
* utils/shd.c (run): Likewise.
* utils/storeinfo.c (main): Likewise.
* pfinet/linux-src/include/net/tcp.h (tcp_clear_xmit_timer): Add
format string for `printk' call which lacked it.
(tcp_timer_is_set): Likewise.
* ufs-fsck/utilities.c (punt): Add format string for `problem' call
which lacked it.
---
nfs/mount.c|4 ++--
pfinet/linux-src/include/net/tcp.h |4 ++--
pfinet/main.c |2 +-
term/main.c|2 +-
ufs-fsck/utilities.c |2 +-
utils/shd.c|2 +-
utils/storeinfo.c |4 ++--
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/nfs/mount.c b/nfs/mount.c
index 07dbb19..6120f87 100644
--- a/nfs/mount.c
+++ b/nfs/mount.c
@@ -199,7 +199,7 @@ mount_root (char *name, char *host)
err = conduct_rpc (&rpcbuf, &p);
if (err)
{
- error (0, err, name);
+ error (0, err, "%s", name);
goto error_with_rpcbuf;
}
/* XXX Protocol spec says this should be a "unix error code"; we'll
@@ -209,7 +209,7 @@ mount_root (char *name, char *host)
p++;
if (err)
{
- error (0, err, name);
+ error (0, err, "%s", name);
goto error_with_rpcbuf;
}
diff --git a/pfinet/linux-src/include/net/tcp.h b/pfinet/linux-src/include/net/tcp.h
index 8072324..abb4b21 100644
--- a/pfinet/linux-src/include/net/tcp.h
+++ b/pfinet/linux-src/include/net/tcp.h
@@ -1066,7 +1066,7 @@ static inline void tcp_clear_xmit_timer(struct sock *sk, int what)
timer = &tp->probe_timer;
break;
default:
- printk(timer_bug_msg);
+ printk("%s", timer_bug_msg);
return;
};
if(timer->prev != NULL)
@@ -1088,7 +1088,7 @@ static inline int tcp_timer_is_set(struct sock *sk, int what)
return tp->probe_timer.prev != NULL;
break;
default:
- printk(timer_bug_msg);
+ printk("%s", timer_bug_msg);
};
return 0;
}
diff --git a/pfinet/main.c b/pfinet/main.c
index b4af267..1357b03 100644
--- a/pfinet/main.c
+++ b/pfinet/main.c
@@ -415,7 +415,7 @@ pfinet_bind (int portclass, const char *name)
}
if (err)
-error (1, err, name);
+error (1, err, "%s", name);
ports_port_deref (cntl);
diff --git a/term/main.c b/term/main.c
index 516a2dc..405e7cd 100644
--- a/term/main.c
+++ b/term/main.c
@@ -405,7 +405,7 @@ main (int argc, char **argv)
}
if (err)
- error (1, err, peer_name);
+ error (1, err, "%s", peer_name);
(*peercntl)->hook = peer_name;
ports_port_deref (*peercntl);
diff --git a/ufs-fsck/utilities.c b/ufs-fsck/utilities.c
index 5e081fe..14705f8 100644
--- a/ufs-fsck/utilities.c
+++ b/ufs-fsck/utilities.c
@@ -270,7 +270,7 @@ retch (char *reason)
static void
punt (char *msg)
{
- problem (0, msg);
+ problem (0, "%s", msg);
flush_problems ();
exit (8);
}
diff --git a/utils/shd.c b/utils/shd.c
index 0587fa4..a1a4b26 100644
--- a/utils/shd.c
+++ b/utils/shd.c
@@ -86,7 +86,7 @@ run (char **argv, int fd0, int fd1)
file = file_name_lookup (program, O_EXEC, 0);
if (file == MACH_PORT_NULL)
{
- error (0, errno, program);
+ error (0, errno, "%s", program);
return -1;
}
else
diff --git a/utils/storeinfo.c b/utils/storeinfo.c
index 411bf11..a738d50 100644
--- a/utils/storeinfo.c
+++ b/utils/storeinfo.c
@@ -204,7 +204,7 @@ main (int argc, char *argv[])
struct store *store;
if (file == MACH_PORT_NULL)
- error (3, err, source);
+ error (3, err, "%s", source);
if (print_prefix < 0)
/* By default, only print filename prefixes for multiple files. */
@@ -220,7 +220,7 @@ main (int argc, char *argv[])
of what the unknown data looked like. */
err = store_create (file, STORE_INACTIVE|STORE_NO_FILEIO, 0, &store);
if (err)
- error (4, err, source);
+ error (4, err, "%s", source);
print_store (store, 0, what);
store_free (store);
--
1.7.6.3
signature.asc
Description: This is a digitally signed message part.
[PATCH] make libdiskfs handle _PC_PATH_MAX
Hi, currently, querying (f)pathconf() for _PC_PATH_MAX return -1 and sets errno = EINVAL. At least to my reading of pathconf() in POSIX, when a variable has no limit pathconf() for it should return -1 and not change errno. For example: $ getconf PATH_MAX . getconf: pathconf: .: Invalid argument fpathconf() just queries the translator which handles the specified path, so the fix I thought about is to make the io_pathconf() reply in libdiskfs handle _PC_PATH_MAX too among the other cases of "set -1 and return 0". With the applied path, I correctly get: $ getconf PATH_MAX . undefined -- Pino Toscano From f7835870d3a2f0447faa31f1159a50a63e26eac9 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Tue, 18 Oct 2011 23:47:51 +0200 Subject: [PATCH] libdiskfs: handle _PC_PATH_MAX in pathconf Explicitly return -1 also for _PC_PATH_MAX to indicate there is no limit for it, otherwise EINVAL is returned for it. * libdiskfs/io-pathconf.c (diskfs_S_io_pathconf): Handle _PC_PATH_MAX too. --- libdiskfs/io-pathconf.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libdiskfs/io-pathconf.c b/libdiskfs/io-pathconf.c index b851f9b..38e277c 100644 --- a/libdiskfs/io-pathconf.c +++ b/libdiskfs/io-pathconf.c @@ -41,6 +41,7 @@ diskfs_S_io_pathconf (struct protid *cred, case _PC_PIPE_BUF: case _PC_VDISABLE: case _PC_SOCK_MAXBUF: +case _PC_PATH_MAX: *value = -1; break; -- 1.7.6.3 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] make libdiskfs handle _PC_PATH_MAX
Hi, Alle mercoledì 19 ottobre 2011, Thomas Schwinge ha scritto: > Just an oversight, or is there a reason to not changing > libnetfs/io-pathconf.c and term/users.c, too? term/users.c already does that, it seems. About libnetfs/io-pathconf.c, I don't have an NF setup, but I guess the attached patch should do it (testing welcome!). -- Pino Toscano From 2ad3fb8ec4969144536b0e1a877971abae4224c3 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Wed, 19 Oct 2011 22:25:34 +0200 Subject: [PATCH] libnetfs: handle _PC_PATH_MAX in pathconf Explicitly return -1 also for _PC_PATH_MAX to indicate there is no limit for it, otherwise EINVAL is returned for it. * libnetfs/io-pathconf.c (netfs_S_io_pathconf): Handle _PC_PATH_MAX too. --- libnetfs/io-pathconf.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/libnetfs/io-pathconf.c b/libnetfs/io-pathconf.c index 78244bc..2fd3f5b 100644 --- a/libnetfs/io-pathconf.c +++ b/libnetfs/io-pathconf.c @@ -39,6 +39,7 @@ netfs_S_io_pathconf (struct protid *user, case _PC_PIPE_BUF: case _PC_VDISABLE: case _PC_SOCK_MAXBUF: +case _PC_PATH_MAX: *value = -1; break; -- 1.7.6.3 signature.asc Description: This is a digitally signed message part.
[PATCH] pthread: fix the stale TSDs when deleting a key
Hi,
as I found few months ago[1], when calling pthread_key_delete() all its
values in threads are not removed; this, plus the the fact that deleted
keys can be reused in pthread_key_create(), causes that in some
occasions (i.e. when a new key is actually a reused one) there are the
old thread specific data still available in threads which previously set
some using the key prior of being deleted and reused again.
Attached there is a patch fixing this behaviour by cleaning up all the
values of the key being deleted. It includes also a small test case.
[1]
http://www.gnu.org/software/hurd/open_issues/libpthread_pthread_key_create_reuse.html
--
Pino Toscano
From 557337b9348f29b29acb4b226fd8ffddf5f22d30 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Wed, 2 Nov 2011 17:38:46 +0100
Subject: [PATCH] Remove all the values when deleting a key
When deleting a key using `pthread_key_delete', delete all the values
associated to that key in all the threads available. Otherwise, the
key reuse in `pthread_key_create' can cause new keys to have thread
specific data of the previously used key with the same index.
Add a test for this case, which creates and deletes pairs of keys
checking that they have a NULL thread specific data after creation.
* sysdeps/hurd/pt-key-delete.c (pthread_key_delete): Remove all the
values of the key in all the threads.
* tests/Makefile (CHECK_SRC): Add test-17.c.
* tests/test-17.c: New file.
---
sysdeps/hurd/pt-key-delete.c | 19 ++
tests/Makefile |2 +-
tests/test-17.c | 57 ++
3 files changed, 77 insertions(+), 1 deletions(-)
create mode 100644 tests/test-17.c
diff --git a/sysdeps/hurd/pt-key-delete.c b/sysdeps/hurd/pt-key-delete.c
index 2426bb1..9d88647 100644
--- a/sysdeps/hurd/pt-key-delete.c
+++ b/sysdeps/hurd/pt-key-delete.c
@@ -35,8 +35,27 @@ pthread_key_delete (pthread_key_t key)
err = EINVAL;
else
{
+ int i;
+
__pthread_key_destructors[key] = PTHREAD_KEY_INVALID;
__pthread_key_invalid_count ++;
+
+ pthread_rwlock_rdlock (&__pthread_threads_lock);
+ for (i = 0; i < __pthread_num_threads; ++i)
+ {
+ struct __pthread *t;
+
+ t = __pthread_threads[i];
+
+ if (t == NULL)
+ continue;
+
+ /* Just remove the key, no need to care whether it was
+ already there. */
+ if (t->thread_specifics)
+ hurd_ihash_remove (t->thread_specifics, key);
+ }
+ pthread_rwlock_unlock (&__pthread_threads_lock);
}
__pthread_mutex_unlock (&__pthread_key_lock);
diff --git a/tests/Makefile b/tests/Makefile
index 9509c95..4e2a4a8 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -4,7 +4,7 @@ LDLIBS = -lpthread
CHECK_SRC := test-1.c test-2.c test-3.c test-6.c test-7.c test-8.c \
test-9.c test-10.c test-11.c test-12.c test-13.c test-14.c \
- test-15.c test-16.c
+ test-15.c test-16.c test-17.c
CHECK_OBJS := $(addsuffix .o,$(basename $(notdir $(CHECK_SRC
CHECK_PROGS := $(basename $(notdir $(CHECK_SRC))) \
diff --git a/tests/test-17.c b/tests/test-17.c
new file mode 100644
index 000..a8bd150
--- /dev/null
+++ b/tests/test-17.c
@@ -0,0 +1,57 @@
+/* Test that the key reuse inside libpthread does not cause thread
+ specific values to persist. */
+
+#define _GNU_SOURCE 1
+
+#include
+#include
+#include
+#include
+
+void
+work (int iter)
+{
+ error_t err;
+ pthread_key_t key1;
+ pthread_key_t key2;
+ void *value1;
+ void *value2;
+
+ printf ("work/%d: start\n", iter);
+ err = pthread_key_create (&key1, NULL);
+ assert (err == 0);
+ err = pthread_key_create (&key2, NULL);
+ assert (err == 0);
+
+ value1 = pthread_getspecific (key1);
+ value2 = pthread_getspecific (key2);
+ printf ("work/%d: pre-setspecific: %p,%p\n", iter, value1, value2);
+ assert (value1 == NULL);
+ assert (value2 == NULL);
+ err = pthread_setspecific (key1, (void *)(0x100 + iter));
+ assert (err == 0);
+ err = pthread_setspecific (key2, (void *)(0x200 + iter));
+ assert (err == 0);
+
+ value1 = pthread_getspecific (key1);
+ value2 = pthread_getspecific (key2);
+ printf ("work/%d: post-setspecific: %p,%p\n", iter, value1, value2);
+ assert (value1 == (void *)(0x100 + iter));
+ assert (value2 == (void *)(0x200 + iter));
+
+ err = pthread_key_delete (key1);
+ assert (err == 0);
+ err = pthread_key_delete (key2);
+ assert (err == 0);
+}
+
+int
+main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 8; ++i)
+work (i + 1);
+
+ return 0;
+}
--
1.7.7
signature.asc
Description: This is a digitally signed message part.
[PATCH] key validity checks in pthread_{get,set}specific
Hi,
attached there is a patch to do validity checks for the pthread key
passed to pthread_getspecific and pthread_setspecific (more details in
the commit log), with two very simple tests for the new checks.
Thanks,
--
Pino Toscano
From e64c1c3f36168593eccb577218971d4fa0448ee0 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Sun, 6 Nov 2011 12:39:04 +0100
Subject: [PATCH] pthread_getspecific, pthread_setspecific: check the key
validity
When getting a TSD, handle gracefully the case of an invalid key.
When setting a TSD, check for the validity of the key as recommended
(although not required) by POSIX. This also avoids potentially
filling the `thread_specifics' hash of threads with TSD of invalid
keys.
Add two simple checks in test-7.c for the two situations above.
* sysdeps/hurd/pt-getspecific.c (pthread_getspecific): Check the
validity of the specified key.
* sysdeps/hurd/pt-setspecific.c (pthread_setspecific): Likewise.
* tests/test-7.c (main): Add two assertions.
---
sysdeps/hurd/pt-getspecific.c |4 +++-
sysdeps/hurd/pt-setspecific.c |4
tests/test-7.c|3 +++
3 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/sysdeps/hurd/pt-getspecific.c b/sysdeps/hurd/pt-getspecific.c
index 3060598..71ec63c 100644
--- a/sysdeps/hurd/pt-getspecific.c
+++ b/sysdeps/hurd/pt-getspecific.c
@@ -27,7 +27,9 @@ pthread_getspecific (pthread_key_t key)
{
struct __pthread *self;
- assert (key < __pthread_key_count);
+ if (key < 0 || key >= __pthread_key_count
+ || __pthread_key_destructors[key] == PTHREAD_KEY_INVALID)
+return NULL;
self = _pthread_self ();
if (! self->thread_specifics)
diff --git a/sysdeps/hurd/pt-setspecific.c b/sysdeps/hurd/pt-setspecific.c
index 89ca4d7..d0b7302 100644
--- a/sysdeps/hurd/pt-setspecific.c
+++ b/sysdeps/hurd/pt-setspecific.c
@@ -28,6 +28,10 @@ pthread_setspecific (pthread_key_t key, const void *value)
error_t err;
struct __pthread *self = _pthread_self ();
+ if (key < 0 || key >= __pthread_key_count
+ || __pthread_key_destructors[key] == PTHREAD_KEY_INVALID)
+return EINVAL;
+
if (! self->thread_specifics)
{
err = hurd_ihash_create (&self->thread_specifics, HURD_IHASH_NO_LOCP);
diff --git a/tests/test-7.c b/tests/test-7.c
index 8159be3..22fb1ca 100644
--- a/tests/test-7.c
+++ b/tests/test-7.c
@@ -42,6 +42,9 @@ main (int argc, char **argv)
assert ((pthread_t) val == pthread_self ());
}
+ assert (pthread_getspecific ((pthread_key_t) 0) == NULL);
+ assert (pthread_setspecific ((pthread_key_t) 0, (void *) 0x1) == EINVAL);
+
for (i = 0; i < KEYS; i ++)
err = pthread_key_create (&key[i], des);
--
1.7.7.1
signature.asc
Description: This is a digitally signed message part.
Re: Some experiences with web browsers...
Alle giovedì 10 novembre 2011, Svante Signell ha scritto: > On Thu, 2011-11-10 at 01:00 +0100, Samuel Thibault wrote: > > Most of these are simply due to dbus, actually. > > What's wrong with it, the latest version, 1.4.16-1, seems to build > OK. But it doesn't work, mainly for two reasons: - missing socket credentials, which prevents user applications to connect to the system bus (see the socket credentials and scm creds pages on the "open issues" wiki page) - select() which behaves wrongly in very short timeouts (see eg [1]); capping the timeout (when specified) to at least 1ms is usually enough Note both these two problems also prevent gamin to work, and the select() issue also may affect other software. [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=79358 -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH 4/4] Add a monotonic time device
Alle sabato 27 agosto 2011, Samuel Thibault ha scritto:
> Roland McGrath, le Sat 27 Aug 2011 10:03:24 -0700, a écrit :
> > > Pino Toscano, le Fri 26 Aug 2011 20:29:43 +0200, a écrit :
> > > > +extern vm_offset_t monotonicmmap();
> > > > +#definemonotonicname "monotonic"
> > >
> > > Ah, I was rather thinking about appending the new clock to the
> > > existing mappable clock. I don't think it is useful to separate
> > > them.
> >
> > It does seem good to consolidate on the one page, no reason to have
> > two pages. But how exactly will the user tell that he has a new
> > kernel that has both? It could be by some magic number on the page
> > or something.
>
> It could simply be the field being non-zero?
Won't that be problematic when running a newer glibc with a older
gnumach that does not zero the page?
> > But
> > perhaps it's better just to have a new device name that's an alias
> > for "time", but only recognized by new microkernels that are
> > supplying the additional words on the same page.
>
> That's another way indeed.
I will try this way, thanks for the hint.
I was thinking that we could combine the new alias hint with an extended
mapped_time_value struct e.g. like this:
struct mapped_time_value_extended {
integer_t seconds;
integer_t microseconds;
integer_t check_seconds;
integer_t mapped_time_magic;
/* new in mapped_time_magic >= 1 */
signed64_t time_nanoseconds;
integer_t monotonic_seconds; /* or signed64_t here too? */
signed64_t monotonic_nanoseconds;
};
so we have a magic value that would indicate extensions, and already
adding fields for nanoseconds precision (so glibc will be able to deal
with them, even if gnumach would not be more precise than microseconds,
for now).
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH, glibc] _hurd_socket_server: check for negative domains
Hi,
attached there is a patch for glibc to check for negative indexes in
_hurd_socket_server(). This should avoid that calls to socket() or
socketpair() with domain < 0 read/set OOB memory (and potentially
crashing, like it happens when doing such calls using perl's socketpair
for example), causing also a warning "task XXX deallocating an invalid
port YYY, most probably a bug" in that situation.
--
Pino Toscano
Refuse negative socket domains right away; otherwise, it is possible to read
and set out-of-bounds locations of the `servers' array (returning the values
at those invalid memory locations), and even try to deallocate ports with
random values if the `dead' parameter is different than zero.
* hurd/hurdsock.c (_hurd_socket_server): Check for negative domains.
--- a/hurd/hurdsock.c
+++ b/hurd/hurdsock.c
@@ -47,6 +47,12 @@
{
socket_t server;
+ if (domain < 0)
+{
+ errno = EAFNOSUPPORT;
+ return MACH_PORT_NULL;
+}
+
HURD_CRITICAL_BEGIN;
__mutex_lock (&lock);
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] _hurd_socket_server: check for negative domains
Alle lunedì 21 novembre 2011, Samuel Thibault ha scritto:
> Pino Toscano, le Mon 21 Nov 2011 01:25:55 +0100, a écrit :
> > attached there is a patch for glibc to check for negative indexes
> > in _hurd_socket_server().
>
> Thanks! Could you submit it to libc-alpha? (with rol...@gnu.org
> CC-ed)
Updated patch in form of git commit.
(I have already the copyright assignment for glibc done.)
--
Pino Toscano
From 4e31da499e47738cee4784b4d14f2164e84fe711 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Mon, 21 Nov 2011 14:21:03 +0100
Subject: [PATCH] hurdsock: reject negative domains
Reject negative socket domains right away; otherwise, it is possible to read
and set out-of-bounds locations of the `servers' array (returning the values
at those invalid memory locations), and even try to deallocate ports with
random values if the `dead' parameter is different than zero.
---
ChangeLog |5 +
hurd/hurdsock.c |6 ++
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index d9866de..472cb7c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-11-21 Pino Toscano
+
+ * hurd/hurdsock.c (_hurd_socket_server): Check for negative domains,
+ and reject them.
+
2011-11-17 Ulrich Drepper
* Makefile.in: Remove CVSOPT handling.
diff --git a/hurd/hurdsock.c b/hurd/hurdsock.c
index a01b8aa..f2817e3 100644
--- a/hurd/hurdsock.c
+++ b/hurd/hurdsock.c
@@ -47,6 +47,12 @@ _hurd_socket_server (int domain, int dead)
{
socket_t server;
+ if (domain < 0)
+{
+ errno = EAFNOSUPPORT;
+ return MACH_PORT_NULL;
+}
+
HURD_CRITICAL_BEGIN;
__mutex_lock (&lock);
--
1.7.7.1
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] hurdsock: reject negative domains
Alle lunedì 21 novembre 2011, Samuel Thibault ha scritto:
> Pino Toscano, le Mon 21 Nov 2011 01:25:55 +0100, a écrit :
> > attached there is a patch for glibc to check for negative indexes
> > in _hurd_socket_server().
>
> Thanks! Could you submit it to libc-alpha? (with rol...@gnu.org
> CC-ed)
Updated patch, hopefully now valid for glibc... (apologies for who gets
it twice)
(I have already the copyright assignment for glibc done.)
--
Pino Toscano
hurdsock: reject negative domains
Reject negative socket domains right away; otherwise, it is possible to read
and set out-of-bounds locations of the `servers' array (returning the values
at those invalid memory locations), and even try to deallocate ports with
random values if the `dead' parameter is different than zero.
2011-11-21 Pino Toscano
* hurd/hurdsock.c (_hurd_socket_server): Check for negative domains,
and reject them.
--- a/hurd/hurdsock.c
+++ b/hurd/hurdsock.c
@@ -47,6 +47,12 @@ _hurd_socket_server (int domain, int dea
{
socket_t server;
+ if (domain < 0)
+{
+ errno = EAFNOSUPPORT;
+ return MACH_PORT_NULL;
+}
+
HURD_CRITICAL_BEGIN;
__mutex_lock (&lock);
signature.asc
Description: This is a digitally signed message part.
SIGLOST in recvfrom()
Hi,
with the attached test source, I get SIGLOST in recvfrom().
Basically what happens in recvfrom() is the following:
- the __socket_recv() RPC returns a MACH_PORT_NULL 'addrport'
- execution goes inside the "if (addr != NULL)"
- the __socket_whatis_address() RPC fails because of the null port (I
presume), so err is MACH_SEND_INVALID_DEST
- the generic "if (err)" is followed, and then __hurd_sockfail() raises
SIGLOST
The question is: is __socket_recv() supposed to actually return an
addrport in this case, or should recvfrom() just being able to
gracefully cope with this situation?
On Linux the address length is set to 0 by recvfrom(), so I guess that
this kind of sockets have no address?
--
Pino Toscano
#include
#include
#include
#include
#include
#include
#include
void die(int x, const char *s)
{
perror(s);
exit(x);
}
int main()
{
int ret;
int p[2];
char buf[2];
char namebuf[256];
socklen_t bufsize = sizeof(namebuf);
ret = socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, p);
if (ret) die(1, "socketpair");
ret = send(p[0], "xyz", 3, 0);
printf("> send: %d\n", ret);
if (ret < 0) die(2, "send");
ret = recvfrom(p[1], buf, sizeof(buf), 0, (struct sockaddr *)namebuf, &bufsize);
printf("> recvfrom: %d, %d\n", ret, bufsize);
if (ret < 0) die(3, "recvfrom");
close(p[0]);
close(p[1]);
return 0;
}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurdsock: reject negative domains
Alle mercoledì 23 novembre 2011, Thomas Schwinge ha scritto:
> Hi!
>
> On Mon, 21 Nov 2011 22:23:26 +0100, Pino Toscano
wrote:
> > Reject negative socket domains right away; otherwise, it is
> > possible to read and set out-of-bounds locations of the `servers'
> > array (returning the values at those invalid memory locations),
> > and even try to deallocate ports with random values if the `dead'
> > parameter is different than zero.
> >
> > 2011-11-21 Pino Toscano
> >
> > * hurd/hurdsock.c (_hurd_socket_server): Check for negative
> > domains, and reject them.
> >
> > --- a/hurd/hurdsock.c
> > +++ b/hurd/hurdsock.c
> > @@ -47,6 +47,12 @@ _hurd_socket_server (int domain, int dea
> >
> > {
> >
> >socket_t server;
> >
> > + if (domain < 0)
> > +{
> > + errno = EAFNOSUPPORT;
> > + return MACH_PORT_NULL;
> > +}
> > +
>
> Thanks; the issue is valid, but we may want to fix it differently:
> _hurd_socket_server is an internal function, and internally we should
> always know what we're doing: that is, should only be calling it with
> valid data, such as the PF_* constants -- which is done in all places
> but socket and socketpair, which happen to be external interfaces.
> Should instead in these two functions the domain parameter be
> checked for validity (and negative ones refused with EINVAL)?
On the other hand, the whole logic of the port cache as array (which
make negative indexes not acceptable) is entirely in hurdsock.c, so IMHO
it wouldn't make sense to know that (internal) logic also outide that
file.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] hurd: recvfrom(): take into account null address ports
Hi,
attached there is a patch for the Hurd recvfrom(). It avoids the
application to get a SIGLOST as a indirect consequence of a null mach
port being used.
Thanks,
--
Pino Toscano
hurd: recvfrom(): take into account null address ports
Some kinds of sockets may return a null address port when calling the
`socket_recv' RPC, so avoid using it in case an address argument buffer is
asked to be filled. In such case, set the length of that address buffer to
zero.
2011-11-26 Pino Toscano
* sysdeps/mach/hurd/recvfrom.c (__recvfrom): Check also for a null
address port. Set `addr_len' to 0 when not filling `addrarg'.
--- a/sysdeps/mach/hurd/recvfrom.c
+++ b/sysdeps/mach/hurd/recvfrom.c
@@ -55,7 +55,7 @@
return __hurd_sockfail (fd, flags, err);
/* Get address data for the returned address port if requested. */
- if (addr != NULL)
+ if (addr != NULL && addrport != MACH_PORT_NULL)
{
char *buf = (char *) addr;
mach_msg_type_number_t buflen = *addr_len;
@@ -89,6 +89,8 @@
if (buflen > 0)
addr->sa_family = type;
}
+ else if (addr_len != NULL)
+*addr_len = 0;
__mach_port_deallocate (__mach_task_self (), addrport);
signature.asc
Description: This is a digitally signed message part.
unfilled path in getsockname() for unix sockets
Hi,
while debugging, I apparently stumbled upon a bug of getsockname() for a
bound unix socket, as also the attached test program shows: sun_path is
left unfilled, as also the comment of S_socket_whatis_address() in
hurd/pflocal/pf.c would suggest.
There's also another possibly weird behaviour in that:
S_socket_whatis_address() sets its sockaddr_len parameter in what seems
also what Linux does, ie
offsetof(struct sockaddr, sa_data) + strlen(sun_path) + 1
instead of sizeof(struct sockaddr_un), like apparently most of other
OSes do[1]; of course, given that sun_path is never filled (as said
above), the returned sockaddr_len is always 3.
[1] in perl's code, I've seen this snippet:
unpack_sockaddr_un(sun_sv)
[...]
# ifndef __linux__
/* On Linux sockaddrlen on sockets returned by accept, recvfrom,
getpeername and getsockname is not equal to sizeof(addr). */
if (sockaddrlen != sizeof(addr)) {
croak("Bad arg length for %s, length is %d, should be %d",
"Socket::unpack_sockaddr_un",
sockaddrlen, sizeof(addr));
}
# endif
--
Pino Toscano
#include
#include
#include
#include
#include
#include
#include
#include
#include
static const char s_socketpath[] = "/tmp/test.sn.sock";
void die(int x, const char *s)
{
perror(s);
unlink(s_socketpath);
exit(x);
}
int main()
{
int ret;
int s;
struct sockaddr_un sock;
char namebuf[256];
socklen_t bufsize = sizeof(namebuf);
namebuf[0] = 'a';
namebuf[1] = 0;
memset(&sock, 0, sizeof(sock));
sock.sun_family = AF_UNIX;
strcpy(sock.sun_path, s_socketpath);
s = socket(AF_UNIX, SOCK_STREAM, PF_UNSPEC);
if (s < 0) die(1, "socket");
ret = bind(s, (struct sockaddr *)&sock, sizeof(sock));
if (ret < 0) die(2, "bind");
ret = getsockname(s, (struct sockaddr *)namebuf, &bufsize);
{
struct sockaddr_un *ss = (struct sockaddr_un *)namebuf;
printf("> getsockname: %d, %d ('%s') vs %u/%u\n", ret, bufsize, namebuf, offsetof(struct sockaddr, sa_data) + strlen(ss->sun_path) + 1,sizeof(sock));
printf("> ... %d vs %d, %s (%u)\n", ss->sun_family, AF_UNIX, ss->sun_path, strlen(ss->sun_path));
}
if (ret < 0) die(3, "getsockname");
shutdown(s, SHUT_RDWR);
close(s);
unlink(s_socketpath);
return 0;
}
signature.asc
Description: This is a digitally signed message part.
Re: unfilled path in getsockname() for unix sockets
Alle domenica 4 dicembre 2011, Samuel Thibault ha scritto:
> Pino Toscano, le Sun 04 Dec 2011 13:14:52 +0100, a écrit :
> > while debugging, I apparently stumbled upon a bug of getsockname()
> > for a bound unix socket, as also the attached test program shows:
> > sun_path is left unfilled, as also the comment of
> > S_socket_whatis_address() in hurd/pflocal/pf.c would suggest.
>
> Which is a design choice, apparently: only glibc's bind.c knows about
> the socket path.
Hmm :-/ And there's really no way to get it later?
> > There's also another possibly weird behaviour in that:
> > S_socket_whatis_address() sets its sockaddr_len parameter in what
> > seems also what Linux does, ie
> >
> > offsetof(struct sockaddr, sa_data) + strlen(sun_path) + 1
> >
> > instead of sizeof(struct sockaddr_un),
>
> Yes, because the size of the sun_path member is rather indicative
> only (and relatively small: 108 only!). Everything should also be
> working with bigger paths, so it does not really make sense to
> return the arbitrary size of sockaddr_un.
Yes, I knew sun_path was rather short, my doubt was whether which was a
"better" return value, as usually socket functions return values of the
whole sockaddr_* structs.
> > [1] in perl's code, I've seen this snippet:
> > unpack_sockaddr_un(sun_sv)
> > [...]
> > # ifndef __linux__
> >
> > /* On Linux sockaddrlen on sockets returned by accept,
> > recvfrom,
> >
> >getpeername and getsockname is not equal to
> >sizeof(addr). */
> >
> > if (sockaddrlen != sizeof(addr)) {
> >
> > croak("Bad arg length for %s, length is %d, should be
> > %d",
> >
> > "Socket::unpack_sockaddr_un",
> > sockaddrlen, sizeof(addr));
> >
> > }
> >
> > # endif
>
> I'd say restrict to !__GNU__ too, then.
I guess so.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH] fix socket creation errnos
Hi, attached there is a patch to fix in pflocal and pfinet the return values for wrong values of socket types and protocols on socket creation. -- Pino Toscano From 904cbb3dbf1478995629f0cfb83713de786c25e8 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Tue, 6 Dec 2011 00:30:30 +0100 Subject: [PATCH] Fix error values on socket creation On socket creation, return the correct errno values, EPROTOTYPE and EPROTONOSUPPORT, for invalid socket types and protocols. * pfinet/socket-ops.c (S_socket_create): Correctly return EPROTOTYPE and EPROTONOSUPPORT. * pflocal/pf.c (S_socket_create): Correctly return EPROTOTYPE. --- pfinet/socket-ops.c | 14 -- pflocal/pf.c|2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pfinet/socket-ops.c b/pfinet/socket-ops.c index 0267542..b4172dc 100644 --- a/pfinet/socket-ops.c +++ b/pfinet/socket-ops.c @@ -51,12 +51,14 @@ S_socket_create (struct trivfs_protid *master, /* Don't allow bogus SOCK_PACKET here. */ - if ((sock_type != SOCK_STREAM - && sock_type != SOCK_DGRAM - && sock_type != SOCK_SEQPACKET - && sock_type != SOCK_RAW) - || protocol < 0) -return EINVAL; + if (sock_type != SOCK_STREAM + && sock_type != SOCK_DGRAM + && sock_type != SOCK_SEQPACKET + && sock_type != SOCK_RAW) +return EPROTOTYPE; + + if (protocol < 0) +return EPROTONOSUPPORT; __mutex_lock (&global_lock); diff --git a/pflocal/pf.c b/pflocal/pf.c index 32c12e1..55824d4 100644 --- a/pflocal/pf.c +++ b/pflocal/pf.c @@ -65,7 +65,7 @@ S_socket_create (mach_port_t pf, case SOCK_SEQPACKET: pipe_class = seqpack_pipe_class; break; default: - return ESOCKTNOSUPPORT; + return EPROTOTYPE; } err = sock_create (pipe_class, mode, &sock); -- 1.7.7.3 signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] _hurd_select: check for invalid parameter values
Hi,
attached there is a patch to add some validation for parameters of
_hurd_select().
Thanks,
--
Pino Toscano
_hurd_select: check for invalid parameter values
Check for invalid values of the `timeout' and `nfds' parameters; move the
calculation of `to' right after the validation of `timeout'.
2011-11-26 Pino Toscano
* hurd/hurdselect.c (_hurd_select): Return EINVAL for negative
`timeout' values.
Return EINVAL for `nfds' values either negative or greater than
FD_SETSIZE.
--- a/hurd/hurdselect.c
+++ b/hurd/hurdselect.c
@@ -50,10 +50,7 @@ _hurd_select (int nfds,
error_t err;
fd_set rfds, wfds, xfds;
int firstfd, lastfd;
- mach_msg_timeout_t to = (timeout != NULL ?
- (timeout->tv_sec * 1000 +
- (timeout->tv_nsec + 99) / 100) :
- 0);
+ mach_msg_timeout_t to = 0;
struct
{
struct hurd_userlink ulink;
@@ -72,6 +69,24 @@ _hurd_select (int nfds,
assert (sizeof (union typeword) == sizeof (mach_msg_type_t));
assert (sizeof (uint32_t) == sizeof (mach_msg_type_t));
+ if (nfds < 0 || nfds > FD_SETSIZE)
+{
+ errno = EINVAL;
+ return -1;
+}
+
+ if (timeout != NULL)
+{
+ if (timeout->tv_sec < 0 || timeout->tv_nsec < 0)
+ {
+ errno = EINVAL;
+ return -1;
+ }
+
+ to = timeout->tv_sec * 1000 +
+ (timeout->tv_nsec + 99) / 100;
+}
+
if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset))
return -1;
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] nanosleep: check for invalid parameter values
Hi,
attached there is a patch to add some validation for parameters of
Mach's nanosleep() (used by Hurd).
Thanks,
--
Pino Toscano
mach: nanosleep: check for invalid parameter values
Check for invalid values of the `requested_time' parameters; move the
calculation of `ms' after the validation of `requested_time'.
2011-12-10 Pino Toscano
* sysdeps/mach/nanosleep.c (__nanosleep): Return EINVAL for negative
seconds or nanoseconds of `requested_time', or for nanoseconds equal
or greater than 10.
--- a/sysdeps/mach/nanosleep.c
+++ b/sysdeps/mach/nanosleep.c
@@ -28,11 +28,19 @@ __nanosleep (const struct timespec *requ
{
mach_port_t recv;
struct timeval before, after;
- const mach_msg_timeout_t ms
-= requested_time->tv_sec * 1000
-+ (requested_time->tv_nsec + 99) / 100;
+ mach_msg_timeout_t ms;
+
+ if (requested_time->tv_sec < 0
+ || requested_time->tv_nsec < 0
+ || requested_time->tv_nsec >= 10)
+{
+ errno = EINVAL;
+ return -1;
+}
recv = __mach_reply_port ();
+ ms = requested_time->tv_sec * 1000
+ + (requested_time->tv_nsec + 99) / 100;
if (remaining && __gettimeofday (&before, NULL) < 0)
return -1;
signature.asc
Description: This is a digitally signed message part.
0-bytes read() blocks for standard fd's while it shouldn't
Hi, while debugging, I discovered a problem a bug in read(): when called with nbyte = 0 for the fd's of stdin/out/err like: ret = read(0, buf, 0); it blocks as if it would wait for data, while it should return 0 in absence of errors. That read() correctly returns 0 for an fd a regular file. I didn't have much success in locating who is replying to the io_read for those fd's, so for now I'll just report the issue. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH] procfs: fix process file name in stat/status
Hi,
Alle sabato 24 dicembre 2011, Guillem Jover ha scritto:
> * If Hurd's procfs interface is supposed to provide a Linux
> compatible output, then it needs to switch to only a program name,
> not a full path in /proc//stat, /statm, etc.
Attached there is a patch (for the jkoenig/master branch) to fix this
behaviour in per-process `stat' and `status' files.
Thanks,
--
Pino Toscano
From 3eec2e26d3c4cf83004eed079f0455ed4485095e Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Sat, 14 Jan 2012 14:47:58 +0100
Subject: [PATCH] PID stat/status: show only the file name of processes
The Linux /proc fs shows only the file name of processes in the
`stat' and `status' files of every process directory, so adapt also
procfs to show process file names.
Add a new `args_filename` function, much similar to GNU's `basename'
but returning the original string also when the resulting file name
is an empty string.
* process.c (args_filename): New function.
(process_file_gc_stat): Wrap the `proc_stat_args' result with
`args_filename'.
(process_file_gc_status): Likewise.
---
process.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/process.c b/process.c
index 6652a4e..17a38ea 100644
--- a/process.c
+++ b/process.c
@@ -81,6 +81,12 @@ static long long int timeval_jiffies (time_value_t tv)
return secs * opt_clk_tck / 100.;
}
+static const char *args_filename (const char *name)
+{
+ char *sp = strrchr (name, '/');
+ return sp != NULL && *(sp + 1) != '\0' ? sp + 1 : name;
+}
+
/* Actual content generators */
static ssize_t
@@ -126,7 +132,7 @@ process_file_gc_stat (struct proc_stat *ps, char **contents)
"%u %u " /* RT priority and policy */
"%llu " /* aggregated block I/O delay */
"\n",
- proc_stat_pid (ps), proc_stat_args (ps), state_char (ps),
+ proc_stat_pid (ps), args_filename (proc_stat_args (ps)), state_char (ps),
pi->ppid, pi->pgrp, pi->session,
0, 0, /* no such thing as a major:minor for ctty */
0, /* no such thing as CLONE_* flags on Hurd */
@@ -178,7 +184,7 @@ process_file_gc_status (struct proc_stat *ps, char **contents)
"VmRSS:\t%8u kB\n"
"VmHWM:\t%8u kB\n" /* ie. resident peak */
"Threads:\t%u\n",
- proc_stat_args (ps),
+ args_filename (proc_stat_args (ps)),
state_string (ps),
proc_stat_pid (ps), /* XXX will need more work for threads */
proc_stat_pid (ps),
--
1.7.7.3
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] socket: fix errno code
Hi, attached there is a small patch to fix the errno value for the "protocol not supported" case, from the non-standard EPFNOSUPPORT to EAFNOSUPPORT. Thanks, -- Pino Toscano 2012-01-22 Pino Toscano * sysdeps/mach/hurd/socket.c (__socket): Return EAFNOSUPPORT instead of the non-standard EPFNOSUPPORT. --- a/sysdeps/mach/hurd/socket.c +++ b/sysdeps/mach/hurd/socket.c @@ -57,7 +57,7 @@ __socket (domain, type, protocol) isn't supported. */ if (err == MACH_SEND_INVALID_DEST || err == MIG_SERVER_DIED || err == MIG_BAD_ID || err == EOPNOTSUPP) -err = EPFNOSUPPORT; +err = EAFNOSUPPORT; if (err) return __hurd_fail (err);
[PATCH] swapon: add -e/--ifexists option
Hi,
I implemented in swapon the -e/--ifexists option close as to what's in
util-linux' swapon. It basically ignores unexisting devices/files when
doing `swapon -a`, i.e. only for the devices found in fstab (not for the
ones passed as arguments to `swapon'.
What do you think?
Could it be making our tools like swapon/mount/etc a bit more compatible
(in term of command line options) with the other OSes' ones a (low
priority) goal?
--
Pino Toscano
From d34079adfadebc5f0e936e92257811676b2ebe88 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Sat, 28 Jan 2012 22:41:53 +0100
Subject: [PATCH] swapon: add -e/--ifexists option
Add the same command line option as in util-linux' swapon to not
consider a error if the device/file of a swap entry in fstab does not
exist (and not when activating a device specified as argument to
`swapon').
* sutils/swapon.c (ifexists, options, parse_opt): Add new variable.
Register the new -e/--ifexists option, and associate the `ifexists'
variable with it.
(swaponoff, main): New argument `skipnotexist'.
Return 0 if `open_store' fails with ENOENT and `skipnotexist' is on.
Adapt `swaponoff' calls with 0 as new argument for command line
arguments, while use the value of `ifexists' for swap entries from
fstab.
---
sutils/swapon.c | 18 ++
1 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/sutils/swapon.c b/sutils/swapon.c
index c0c773b..13e4e64 100644
--- a/sutils/swapon.c
+++ b/sutils/swapon.c
@@ -42,12 +42,14 @@ const char *argp_program_version = STANDARD_HURD_VERSION (swapoff);
const char *argp_program_version = STANDARD_HURD_VERSION (swapon);
#endif
-static int ignore_signature, require_signature, quiet;
+static int ignore_signature, require_signature, quiet, ifexists;
static struct argp_option options[] =
{
{"standard", 'a', 0, 0,
"Use all devices marked as `swap' in " _PATH_MNTTAB},
+ {"ifexists",'e', 0, 0,
+ "Silently skip devices that do not exist"},
{"no-signature",'n', 0, 0,
"Do not check for a Linux swap signature page"},
{"require-signature", 's', 0, 0,
@@ -319,7 +321,7 @@ check_signature (const char *name, struct store **storep, int no_remap,
/* Process a single argument file. */
static int
-swaponoff (const char *file, int add)
+swaponoff (const char *file, int add, int skipnotexist)
{
error_t err;
struct store *store;
@@ -332,6 +334,10 @@ swaponoff (const char *file, int add)
err = store_open (file, 0, 0, &store);
if (err)
{
+ /* If the device does not exist but we were told to ignore such error,
+ return cleanly. */
+ if (err == ENOENT && skipnotexist)
+ return 0;
error (0, err, "%s", file);
return err;
}
@@ -467,6 +473,10 @@ main (int argc, char *argv[])
do_all = 1;
break;
+ case 'e':
+ ifexists = 1;
+ break;
+
case 'n':
ignore_signature = 1;
break;
@@ -486,7 +496,7 @@ main (int argc, char *argv[])
#else
#define ONOFF 1
#endif
- swaponoff (arg, ONOFF);
+ swaponoff (arg, ONOFF, 0);
break;
default:
@@ -523,7 +533,7 @@ main (int argc, char *argv[])
{
done = 1;
- err |= swaponoff (me->mnt_fsname, ONOFF);
+ err |= swaponoff (me->mnt_fsname, ONOFF, ifexists);
}
if (done == 0)
error (2, 0, "No swap partitions found in %s", _PATH_MNTTAB);
--
1.7.8.3
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH 0/4] Monotonic clock device in GNU Mach
Hi,
bringing this again, a couple of patches in "dirty form" to get further
review.
Attached there is my new attempt as it was previously discussed, i.e.
providing more data in the kernel page of mapped time.
* 01_types.diff:
This adds a new structure in time_value.h, making it named in a similar
way of stdlib' struct timespec (with the macros of the struct time_value
provided also for time_spec). I chose to provide a new one, yet
basically the same as time_value, as it would be awkward to use a
time_value_t to store nanoseconds in its microseconds member...
* 02_clock.diff:
This patch actually adds the actual new clock, taking care of adding
microseconds to it, and switching the mapped time struct to the new
_extended one.
As usual, comments, remarks, etc are welcome.
(I will provide clean git commits with proper changelog once the work i
finalized.)
Thanks,
--
Pino Toscano
--- a/include/mach/time_value.h
+++ b/include/mach/time_value.h
@@ -63,6 +63,39 @@
}
/*
+ * Extended time specification.
+ */
+
+struct time_spec {
+ integer_t seconds;
+ integer_t nanoseconds;
+};
+typedef struct time_spec time_spec_t;
+
+/*
+ * Macros to manipulate time specs. Assume that time values
+ * are normalized (nanoseconds <= 9).
+ */
+#define TIME_NANOS_MAX (10)
+
+#define time_spec_add_nsec(val, nanos) { \
+ if (((val)->nanoseconds += (nanos)) \
+ >= TIME_NANOS_MAX) { \
+ (val)->nanoseconds -= TIME_NANOS_MAX; \
+ (val)->seconds++;\
+ } \
+}
+
+#define time_spec_add(result, addend) { \
+ (result)->nanoseconds += (addend)->nanoseconds; \
+ (result)->seconds += (addend)->seconds; \
+ if ((result)->nanoseconds >= TIME_NANOS_MAX) { \
+ (result)->nanoseconds -= TIME_NANOS_MAX; \
+ (result)->seconds++;\
+ } \
+}
+
+/*
* Time value available through the mapped-time interface.
* Read this mapped value with
* do {
@@ -77,4 +110,14 @@
integer_t check_seconds;
} mapped_time_value_t;
+typedef struct mapped_time_value_extended {
+ integer_t seconds;
+ integer_t microseconds;
+ integer_t check_seconds;
+ integer_t version;
+ /* new in version >= 1 */
+ time_spec_t clock_rt; /* realtime clock */
+ time_spec_t clock_mt; /* monotonic clock */
+} mapped_time_value_extended_t;
+
#endif /* _MACH_TIME_VALUE_H_ */
--- a/kern/mach_clock.c
+++ b/kern/mach_clock.c
@@ -69,6 +69,7 @@
int hz = HZ; /* number of ticks per second */
int tick = (100 / HZ); /* number of usec per tick */
time_value_t time = { 0, 0 }; /* time since bootup (uncorrected) */
+time_value_t monotonic = { 0, 0 }; /* time since bootup (uncorrected) */
unsigned long elapsed_ticks = 0; /* ticks elapsed since bootup */
int timedelta = 0;
@@ -93,9 +94,9 @@
* We have to insert write fence operations.) FIXME
*/
-mapped_time_value_t *mtime = 0;
+mapped_time_value_extended_t *mtime = 0;
-#define update_mapped_time(time)\
+#define update_mapped_time(time, monotonic) \
MACRO_BEGIN \
if (mtime != 0) { \
mtime->check_seconds = (time)->seconds; \
@@ -103,6 +104,16 @@
mtime->microseconds = (time)->microseconds; \
asm volatile("":::"memory"); \
mtime->seconds = (time)->seconds; \
+ asm volatile("":::"memory"); \
+ mtime->clock_rt.seconds = (time)->seconds; \
+ asm volatile("":::"memory"); \
+ mtime->clock_rt.nanoseconds = \
+ (time)->microseconds * 1000; \
+ asm volatile("":::"memory"); \
+ mtime->clock_mt.seconds = (monotonic)->seconds; \
+ asm volatile("":::"memory"); \
+ mtime->clock_mt.nanoseconds = \
+ (monotonic)->microseconds * 1000; \
} \
MACRO_END
@@ -219,6 +230,7 @@
*/
if (timedelta == 0) {
time_value_add_usec(&time, usec);
+ time_value_add_usec(&monotonic, usec);
}
else {
register int delta;
@@ -232,8 +244,9 @@
timedelta -= tickdelta;
}
time_value_add_usec(&time, delta);
+ time_value_add_usec(&monotonic, delta);
}
- update_mapped_time(&time);
+ update_mapped_time(&time, &monotonic);
/*
* Schedule soft-interupt for timeout if needed
@@ -427,7 +440,7 @@
s = splhigh();
time = new_time;
- update_mapped_time(&time);
+ update_mapped_time(&time, &monotonic);
resettodr();
splx(s);
@@ -498,7 +511,8 @@
!= KERN_SUCCESS)
panic("mapable_time_init");
memset(mtime, 0, PAGE_SIZE);
- update_mapped_time(&time);
+ mtime->version = 1;
+ update_mapped_time(&time, &monotonic);
}
int timeopen()
signature.asc
Description: This is a digitally signed message part.
[PATCH] libpthread: use monotonic clock if present
Hi,
attached there are few commits for libpthread which allow to use
monotonic clock, if present, for pthread conditions.
Currently they should have "no effect", since there's no monotonic
clock, yet, but will allow to automatically use it once available,
detecting it at runtime.
The only bit I'm not too sure is the linking to rt in case libpthread is
compiled a glibc addon (as result of the changes by Samuel an hour ago).
--
Pino Toscano
From d0c2da9c229d90c2301f2263c0b8856ceaf9f517 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Sun, 22 Apr 2012 00:22:47 +0200
Subject: [PATCH 1/3] __pthread_timedblock: add an argument for the clock id
To make `__pthread_timedblock' properly measure time using the right
clock, add a new argument representing the clock to use.
* pthread/pt-internal.h (__pthread_timedblock): New argument CLOCK_ID.
* sysdeps/l4/pt-timedblock.c (__pthread_timedblock): Likewise.
* sysdeps/mach/pt-timedblock.c (__pthread_timedblock): Likewise.
* sysdeps/generic/pt-cond-timedwait.c
(__pthread_cond_timedwait_internal): Pass the clock of the
`pthread_cond' to `__pthread_timedblock'.
* sysdeps/generic/pt-mutex-timedlock.c
(__pthread_mutex_timedlock_internal): Pass CLOCK_REALTIME to
`__pthread_timedblock'.
* sysdeps/generic/pt-rwlock-timedrdlock.c
(__pthread_rwlock_timedrdlock_internal): Likewise.
* sysdeps/generic/pt-rwlock-timedwrlock.c
(__pthread_rwlock_timedwrlock_internal): Likewise.
* sysdeps/generic/sem-timedwait.c (__sem_timedwait_internal):
Likewise.
---
pthread/pt-internal.h |3 ++-
sysdeps/generic/pt-cond-timedwait.c |5 -
sysdeps/generic/pt-mutex-timedlock.c|2 +-
sysdeps/generic/pt-rwlock-timedrdlock.c |2 +-
sysdeps/generic/pt-rwlock-timedwrlock.c |2 +-
sysdeps/generic/sem-timedwait.c |2 +-
sysdeps/l4/pt-timedblock.c |3 ++-
sysdeps/mach/pt-timedblock.c|3 ++-
8 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/pthread/pt-internal.h b/pthread/pt-internal.h
index 6186c86..a1e90aa 100644
--- a/pthread/pt-internal.h
+++ b/pthread/pt-internal.h
@@ -254,7 +254,8 @@ extern void __pthread_block (struct __pthread *thread);
/* Block THREAD until *ABSTIME is reached. */
extern error_t __pthread_timedblock (struct __pthread *__restrict thread,
- const struct timespec *__restrict abstime);
+ const struct timespec *__restrict abstime,
+ clockid_t clock_id);
/* Wakeup THREAD. */
extern void __pthread_wakeup (struct __pthread *thread);
diff --git a/sysdeps/generic/pt-cond-timedwait.c b/sysdeps/generic/pt-cond-timedwait.c
index 483f277..56eb1ec 100644
--- a/sysdeps/generic/pt-cond-timedwait.c
+++ b/sysdeps/generic/pt-cond-timedwait.c
@@ -46,6 +46,7 @@ __pthread_cond_timedwait_internal (pthread_cond_t *cond,
{
error_t err;
int canceltype;
+ clockid_t clock_id = __pthread_default_condattr.clock;
void cleanup (void *arg)
{
@@ -68,6 +69,8 @@ __pthread_cond_timedwait_internal (pthread_cond_t *cond,
/* Add ourselves to the list of waiters. */
__pthread_spin_lock (&cond->__lock);
__pthread_enqueue (&cond->__queue, self);
+ if (cond->__attr)
+clock_id = cond->__attr->clock;
__pthread_spin_unlock (&cond->__lock);
__pthread_mutex_unlock (mutex);
@@ -79,7 +82,7 @@ __pthread_cond_timedwait_internal (pthread_cond_t *cond,
if (abstime)
{
- err = __pthread_timedblock (self, abstime);
+ err = __pthread_timedblock (self, abstime, clock_id);
if (err)
/* We timed out. We may need to disconnect ourself from the
waiter queue.
diff --git a/sysdeps/generic/pt-mutex-timedlock.c b/sysdeps/generic/pt-mutex-timedlock.c
index 883e50a..48bffaf 100644
--- a/sysdeps/generic/pt-mutex-timedlock.c
+++ b/sysdeps/generic/pt-mutex-timedlock.c
@@ -130,7 +130,7 @@ __pthread_mutex_timedlock_internal (struct __pthread_mutex *mutex,
{
error_t err;
- err = __pthread_timedblock (self, abstime);
+ err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
if (err)
/* We timed out. We may need to disconnect ourself from the
waiter queue.
diff --git a/sysdeps/generic/pt-rwlock-timedrdlock.c b/sysdeps/generic/pt-rwlock-timedrdlock.c
index ba610fa..a110213 100644
--- a/sysdeps/generic/pt-rwlock-timedrdlock.c
+++ b/sysdeps/generic/pt-rwlock-timedrdlock.c
@@ -73,7 +73,7 @@ __pthread_rwlock_timedrdlock_internal (struct __pthread_rwlock *rwlock,
{
error_t err;
- err = __pthread_timedblock (self, abstime);
+ err = __pthread_timedblock (self, abstime, CLOCK_REALTIME);
if (err)
/* We timed out. We may need to disconnect ourself from the
waiter queue.
diff --git a/sysdeps/generic/pt-rwlock-timedwrlock.c b/sysdeps/generic/pt-rwlock-timedwrlock.c
index 04eab51..a5cc579 100644
--- a/sysdeps/generic/pt-rwlock-timedwrlock.c
+++ b/sysdeps/generic/pt-rwlock-timedwrlock.c
@@ -59,7 +
[PATCH,HURD] hurd: compliance fixes for getgroups
Hi, attached there is a patch to fix a couple of issues with Hurd's getgroups(), namely when the requested number is < 0 or when it is > 0 but less than the actual number of groups that would be returned (this case is handled by simply returning the first n groups, instead of failing). Thanks, -- Pino Toscano hurd: compliance fixes for getgroups Fail with EINVAL when the requested number of groups is negative, or when it is positive but less than the actual number of groups. 2012-04-27 Pino Toscano * sysdeps/mach/hurd/getgroups.c (__getgroups): Return -1 and set EINVAL for negative `n' or less than `ngids'. --- a/sysdeps/mach/hurd/getgroups.c +++ b/sysdeps/mach/hurd/getgroups.c @@ -30,6 +30,9 @@ __getgroups (n, gidset) int ngids; void *crit; + if (n < 0) +return __hurd_fail (EINVAL); + crit = _hurd_critical_section_lock (); __mutex_lock (&_hurd_id.lock); @@ -53,7 +56,7 @@ __getgroups (n, gidset) /* Now that the lock is released, we can safely copy the group set into the user's array, which might fault. */ if (ngids > n) - ngids = n; + return __hurd_fail (EINVAL); memcpy (gidset, gids, ngids * sizeof (gid_t)); } else signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] hurd: compliance fixes for getlogin_r
Hi,
attached there is a patch to fix few issues in Hurd's getlogin_r():
avoid a static buffer, and check for buffer shorter than necessary.
Thanks,
--
Pino Toscano
hurd: compliance fixes for getlogin_r
* do not make `login' static, as it would make getlogin_r no more reentrant
* fail with ERANGE if the `name' buffer has not enough space for the actual
login string
* make sure to copy with strncpy only the chars of the string
2012-04-27 Pino Toscano
* sysdeps/mach/hurd/getlogin_r.c (getlogin_r): Do not make `login'
static. Return -1 and set ERANGE if `name' is not big enough.
--- a/sysdeps/mach/hurd/getlogin_r.c
+++ b/sysdeps/mach/hurd/getlogin_r.c
@@ -29,13 +29,22 @@ getlogin_r (name, name_len)
char *name;
size_t name_len;
{
- static char login[1024]; /* XXX */
+ char login[1024]; /* XXX */
error_t err;
+ size_t len;
+ login[0] = '\0';
if (err = __USEPORT (PROC, __proc_getlogin (port, login)))
return errno = err;
- strncpy (name, login, name_len);
+ len = strlen (login) + 1;
+ if (len > name_len)
+{
+ errno = ERANGE;
+ return errno;
+}
+
+ strncpy (name, login, len);
return 0;
}
libc_hidden_def (getlogin_r)
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] hurd: compliance fixes for ptsname_r
Hi,
attached there is a patch to fix few issues in Hurd's ptsname_r(),
mostly checking for more error conditions and making sure to set as
errno and return the proper values on error conditions.
Thanks,
--
Pino Toscano
hurd: compliance fixes for ptsname_r
ptsname_r on failure returns the value that is also set as errno; furthermore,
add more checks to it.
* set errno to EINVAL and return it if `buf' is NULL
* set errno and return it on __term_get_peername failure
* set errno to ERANGE other than returning it
In ptsname do not set errno manually, since ptsname_r has set it already.
2012-04-27 Pino Toscano
* sysdeps/mach/hurd/ptsname.c (ptsname): Do not manually set errno.
(__ptsname_r): Fail with EINVAL if `buf' is NULL. Set errno and return
it on __term_get_peername failure. Set errno when `buf' is too short.
--- a/sysdeps/mach/hurd/ptsname.c
+++ b/sysdeps/mach/hurd/ptsname.c
@@ -33,8 +33,6 @@ ptsname (int fd)
error_t err;
err = __ptsname_r (fd, peername, sizeof (peername));
- if (err)
-__set_errno (err);
return err ? NULL : peername;
}
@@ -50,13 +48,22 @@ __ptsname_r (int fd, char *buf, size_t b
size_t len;
error_t err;
+ if (!buf)
+{
+ errno = EINVAL;
+ return EINVAL;
+}
+
peername[0] = '\0';
if (err = HURD_DPORT_USE (fd, __term_get_peername (port, peername)))
-return _hurd_fd_error (fd, err);
+return __hurd_dfail (fd, err), errno;
len = strlen (peername) + 1;
if (len > buflen)
-return ERANGE;
+{
+ errno = ERANGE;
+ return ERANGE;
+}
memcpy (buf, peername, len);
return 0;
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurd: compliance fixes for getgroups
Alle venerdì 27 aprile 2012, Roland McGrath ha scritto: > > 2012-04-27 Pino Toscano > > > > * sysdeps/mach/hurd/getgroups.c (__getgroups): Return -1 and set > > EINVAL for negative `n' or less than `ngids'. > > The norm is to use all caps and no quotes to mention local variable > names. You can also drop the (function) when it's the only function > in the file of the same name. Ah ok, fixed patch attached. Where can I read about these two change log style settings (i.e. variables as all caps, and no (function) when it's the only one in a file)? I don't remember them in the official pages[1], so maybe I missed some documentation... [1] http://www.gnu.org/prep/standards/html_node/Change-Logs.html > Since I see you already have > copyright paperwork and you're doing more than just the one change, > would you like to get set up to commit yourself? Given also my contributions so far, I guess that I should better need review for now, no? -- Pino Toscano hurd: compliance fixes for getgroups Fail with EINVAL when the requested number of groups is negative, or when it is positive but less than the actual number of groups. 2012-04-28 Pino Toscano * sysdeps/mach/hurd/getgroups.c: Return -1 and set EINVAL for negative N or less than NGIDS. --- a/sysdeps/mach/hurd/getgroups.c +++ b/sysdeps/mach/hurd/getgroups.c @@ -31,6 +31,9 @@ int ngids; void *crit; + if (n < 0) +return __hurd_fail (EINVAL); + crit = _hurd_critical_section_lock (); __mutex_lock (&_hurd_id.lock); @@ -54,7 +57,7 @@ /* Now that the lock is released, we can safely copy the group set into the user's array, which might fault. */ if (ngids > n) - ngids = n; + return __hurd_fail (EINVAL); memcpy (gidset, gids, ngids * sizeof (gid_t)); } else signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurd: compliance fixes for getlogin_r
Alle sabato 28 aprile 2012, Roland McGrath ha scritto:
> You can just use string_t and no need for the XXX comment.
Fixed. (I initially made my changes as less intrusive as possible.)
> libc code can use C99 freely these days, so use an inline
> initializing declaration rather than pre-declaring a new variable.
Hm ok; is it bad if I leave them as it is, for now?
> I don't think there's any need to iniitalize the result buffer. We
> trust the RPC stubs to return a properly-terminated string on
> success, and if they didn't then that wouldn't necessarily catch it
> anyway. If you want that sort of paranoia, use __strnlen (login,
> sizeof login - 1).
>
> If you've already called strlen/strnlen then don't use strncpy,
> just use memcpy with the known length.
Fixed.
Attached there is the updated patch.
--
Pino Toscano
hurd: compliance fixes for getlogin_r
* do not make `login' static, as it would make getlogin_r no more reentrant;
change its type to string_t.
* fail with ERANGE if the `name' buffer has not enough space for the actual
login string
* copy with memcpy only the chars of the string
2012-04-28 Pino Toscano
* sysdeps/mach/hurd/getlogin_r.c: Make LOGIN static and as string_t.
Return -1 and set ERANGE if NAME is not big enough.
--- a/sysdeps/mach/hurd/getlogin_r.c
+++ b/sysdeps/mach/hurd/getlogin_r.c
@@ -30,13 +30,21 @@
char *name;
size_t name_len;
{
- static char login[1024]; /* XXX */
+ string_t login;
error_t err;
+ size_t len;
if (err = __USEPORT (PROC, __proc_getlogin (port, login)))
return errno = err;
- strncpy (name, login, name_len);
+ len = __strnlen (login, sizeof login - 1) + 1;
+ if (len > name_len)
+{
+ errno = ERANGE;
+ return errno;
+}
+
+ memcpy (name, login, len);
return 0;
}
libc_hidden_def (getlogin_r)
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurd: compliance fixes for ptsname_r
Alle sabato 28 aprile 2012, Roland McGrath ha scritto:
> Compliance with what? Is there a standard that specifies ptsname_r?
It is a Linux-specific function is implemented by GNU libc and other
systems.
> We don't use implicit boolean coercions, we use (buf != NULL).
Noted.
> But unless there is a standard requiring that you diagnose a
> null pointer argument somehow, then it's actually better that
> it just crash.
Ok, I see that its `buf' argument is marked nonnull. I added that check
because I saw the gnulib test for it explicitly checking that
ptsname_r(fd, NULL, 0) would be properly failing with EINVAL (and the
man page even explicitly mention that return value, unlike with
basically most of the other functions). Should gnulib do that check only
on Linux, then?
Attached the updated patch, also with the hits of the other reviews.
Thanks,
--
Pino Toscano
hurd: fixes for ptsname_r
ptsname_r on failure returns the value that is also set as errno; furthermore,
add more checks to it.
* set errno and return it on __term_get_peername failure
* set errno to ERANGE other than returning it
Also, change the type of `peername' to string_t, and check its length with
__strnlen.
In ptsname do not set errno manually, since ptsname_r has set it already.
2012-04-28 Pino Toscano
* sysdeps/mach/hurd/ptsname.c (ptsname): Do not manually set errno.
(__ptsname_r): Set errno and return it on __term_get_peername failure.
Set errno if the buffer is too short. Make PEERNAME a string_t,
and check its length with __strnlen.
--- a/sysdeps/mach/hurd/ptsname.c
+++ b/sysdeps/mach/hurd/ptsname.c
@@ -35,8 +35,6 @@
error_t err;
err = __ptsname_r (fd, peername, sizeof (peername));
- if (err)
-__set_errno (err);
return err ? NULL : peername;
}
@@ -47,17 +45,19 @@
int
__ptsname_internal (int fd, char *buf, size_t buflen, struct stat64 *stp)
{
- char peername[1024]; /* XXX */
+ string_t peername;
size_t len;
error_t err;
- peername[0] = '\0';
if (err = HURD_DPORT_USE (fd, __term_get_peername (port, peername)))
-return _hurd_fd_error (fd, err);
+return __hurd_dfail (fd, err), errno;
- len = strlen (peername) + 1;
+ len = __strnlen (peername, sizeof peername - 1) + 1;
if (len > buflen)
-return ERANGE;
+{
+ errno = ERANGE;
+ return ERANGE;
+}
memcpy (buf, peername, len);
return 0;
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] nanosleep: check for invalid parameter values
Alle sabato 10 dicembre 2011, Pino Toscano ha scritto: > attached there is a patch to add some validation for parameters of > Mach's nanosleep() (used by Hurd). Disregard this patch -- I'll send a better one with more fixes for nanosleep. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] mach: compliance fixes for nanosleep
Hi,
attached there is a patch to make Mach's nanosleep() (used by Hurd) a
bit more POSIX compliant:
* check for validity of the `requested_time' parameter
* return EINTR when interrupted
* modify correctly the `remaining' parameter only when interrupted
Thanks,
--
Pino Toscano
mach: compliance fixes for nanosleep
Make the nanosleep Mach implementation behave a bit more POSIX compliant:
* check for invalid values of the `requested_time' parameter
* check for an interrupted __mach_msg call, and only in that case modify
`remaining' with the remaining time (correctly calculated, as before it was
set to the elapsed time); fail with EINTR in this case
2012-04-28 Pino Toscano
* sysdeps/mach/nanosleep.c: Return EINVAL for invalid values of
`requested_time'. Properly set the remaining time and return EINTR
if interrupted.
--- a/sysdeps/mach/nanosleep.c
+++ b/sysdeps/mach/nanosleep.c
@@ -27,24 +27,38 @@ __nanosleep (const struct timespec *requ
{
mach_port_t recv;
struct timeval before, after;
+
+ if (requested_time->tv_sec < 0
+ || requested_time->tv_nsec < 0
+ || requested_time->tv_nsec >= 10)
+{
+ errno = EINVAL;
+ return -1;
+}
+
const mach_msg_timeout_t ms
= requested_time->tv_sec * 1000
+ (requested_time->tv_nsec + 99) / 100;
-
recv = __mach_reply_port ();
if (remaining && __gettimeofday (&before, NULL) < 0)
return -1;
- (void) __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
- 0, 0, recv, ms, MACH_PORT_NULL);
+ error_t err = __mach_msg (NULL, MACH_RCV_MSG|MACH_RCV_TIMEOUT|MACH_RCV_INTERRUPT,
+ 0, 0, recv, ms, MACH_PORT_NULL);
__mach_port_destroy (mach_task_self (), recv);
- if (remaining && __gettimeofday (&after, NULL) < 0)
-return -1;
-
- if (remaining)
+ if (err == EMACH_RCV_INTERRUPTED)
{
- timersub (&after, &before, &after);
- TIMEVAL_TO_TIMESPEC (&after, remaining);
+ if (remaining && __gettimeofday (&after, NULL) >= 0)
+ {
+ struct timeval req_time, elapsed, rem;
+ TIMESPEC_TO_TIMEVAL (&req_time, requested_time);
+ timersub (&after, &before, &elapsed);
+ timersub (&req_time, &elapsed, &rem);
+ TIMEVAL_TO_TIMESPEC (&rem, remaining);
+ }
+
+ errno = EINTR;
+ return -1;
}
return 0;
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] sendto: do not crash when addr is NULL
Hi,
currently, sendto() crashes when the specified addr is NULL; the
attached patch fixes it making sendto() with NULL addr behave as it
would be a plain send().
Thanks,
--
Pino Toscano
Hurd: sendto: do not crash when addr is NULL
Work also when the specified addr is NULL; simplify also the usage of err.
2012-05-20 Pino Toscano
* sysdeps/mach/hurd/sendto.c: Consider also when addr is NULL.
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -33,11 +33,11 @@ __sendto (int fd,
const struct sockaddr_un *addr,
socklen_t addr_len)
{
- addr_port_t aport;
- error_t err;
+ addr_port_t aport = MACH_PORT_NULL;
+ error_t err = 0;
size_t wrote;
- if (addr->sun_family == AF_LOCAL)
+ if (addr != NULL && addr->sun_family == AF_LOCAL)
{
/* For the local domain, we must look up the name as a file and talk
to it with the ifsock protocol. */
@@ -52,13 +52,11 @@ __sendto (int fd,
if (err)
return __hurd_fail (err);
}
- else
-err = EIEIO;
/* Get an address port for the desired destination address. */
err = HURD_DPORT_USE (fd,
({
- if (err)
+ if (aport == MACH_PORT_NULL && addr != NULL)
err = __socket_create_address (port,
addr->sun_family,
(char *) addr,
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] sendto: do not crash when addr is NULL
Hi,
Alle lunedì 21 maggio 2012, Roland McGrath ha scritto:
> Put the name of a local variable in all caps when mentioning it in
> the log entry or comments.
Noted, and done.
> It looks like the existing code also has a leak of APORT in the case
> where HURD_DPORT_USE fails (i.e. invalid fd). So you should
> reorganize in some way that fixes that too.
Done.
> It might be simplest just to move the creation of the address port
> (both the AF_LOCAL case and the default case) into a subroutine and
> just call that inside HURD_DPORT_USE. It means that the descriptor
> structure and port are kept alive around the AF_LOCAL case's calls,
> but that seems OK.
Done.
Updated patch attached.
--
Pino Toscano
Hurd: sendto: do not crash when ADDR is NULL
Create a new create_address_port subroutine to isolate the address port creation
(for both local and remove sockets), and use it inside HURD_DPORT_USE.
Also intialize APORT to MACH_PORT_NULL and make sure to always deallocate it.
2012-06-07 Pino Toscano
* sysdeps/mach/hurd/sendto.c (create_address_port): New subroutine.
(__sendto): Use create_address_port. Initialize APORT and always
deallocate it.
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -33,37 +33,50 @@ __sendto (int fd,
const struct sockaddr_un *addr,
socklen_t addr_len)
{
- addr_port_t aport;
+ addr_port_t aport = MACH_PORT_NULL;
error_t err;
size_t wrote;
- if (addr->sun_family == AF_LOCAL)
+ /* Get an address port for the desired destination address. */
+ error_t create_address_port (io_t port,
+ const struct sockaddr_un *addr,
+ socklen_t addr_len,
+ addr_port_t *aport)
{
- /* For the local domain, we must look up the name as a file and talk
- to it with the ifsock protocol. */
- file_t file = __file_name_lookup (addr->sun_path, 0, 0);
- if (file == MACH_PORT_NULL)
- return -1;
- err = __ifsock_getsockaddr (file, &aport);
- __mach_port_deallocate (__mach_task_self (), file);
- if (err == MIG_BAD_ID || err == EOPNOTSUPP)
- /* The file did not grok the ifsock protocol. */
- err = ENOTSOCK;
- if (err)
- return __hurd_fail (err);
+ error_t err;
+
+ if (addr->sun_family == AF_LOCAL)
+ {
+ /* For the local domain, we must look up the name as a file and talk
+ to it with the ifsock protocol. */
+ file_t file = __file_name_lookup (addr->sun_path, 0, 0);
+ if (file == MACH_PORT_NULL)
+ return errno;
+ err = __ifsock_getsockaddr (file, aport);
+ __mach_port_deallocate (__mach_task_self (), file);
+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ /* The file did not grok the ifsock protocol. */
+ err = ENOTSOCK;
+ }
+ else
+ {
+ err = __socket_create_address (port,
+ addr->sun_family,
+ (char *) addr,
+ addr_len,
+ aport);
+ }
+
+ return err;
}
- else
-err = EIEIO;
- /* Get an address port for the desired destination address. */
err = HURD_DPORT_USE (fd,
({
- if (err)
- err = __socket_create_address (port,
- addr->sun_family,
- (char *) addr,
- addr_len,
- &aport);
+ if (addr != NULL)
+ err = create_address_port (port, addr, addr_len,
+ &aport);
+ else
+ err = 0;
if (! err)
{
/* Send the data. */
@@ -72,12 +85,12 @@ __sendto (int fd,
NULL,
MACH_MSG_TYPE_COPY_SEND, 0,
NULL, 0, &wrote);
- __mach_port_deallocate (__mach_task_self (),
- aport);
}
err;
}));
+ __mach_port_deallocate (__mach_task_self (), aport);
+
return err ? __hurd_sockfail (fd, flags, err) : wrote;
}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] sendto: do not crash when addr is NULL
Hi,
Alle martedì 12 giugno 2012, Roland McGrath ha scritto:
> That's almost right, but you should avoid calling
> __mach_port_deallocate if APORT is MACH_PORT_NULL. Also, avoid
> shadowing the local variable ERR with one of the same name in the
> subfunction.
Hm ok, I saw other cases of shadowing by internal subroutines, and
assumed it was okay (since inside them you want to refer to the local
"err" instead of the outermost one.
Thanks for the further review, patch attached.
(I would use the non-ChangeLog lines of the patch header as commit log,
would that be okay?)
--
Pino Toscano
Hurd: sendto: do not crash when ADDR is NULL
Create a new create_address_port subroutine to isolate the address port creation
(for both local and remove sockets), and use it inside HURD_DPORT_USE.
Also intialize APORT to MACH_PORT_NULL and make sure to always deallocate it,
when not null.
2012-06-13 Pino Toscano
* sysdeps/mach/hurd/sendto.c (create_address_port): New subroutine.
(__sendto): Use create_address_port. Initialize APORT and deallocate
it if not null.
--- a/sysdeps/mach/hurd/sendto.c
+++ b/sysdeps/mach/hurd/sendto.c
@@ -33,37 +33,50 @@ __sendto (int fd,
const struct sockaddr_un *addr,
socklen_t addr_len)
{
- addr_port_t aport;
+ addr_port_t aport = MACH_PORT_NULL;
error_t err;
size_t wrote;
- if (addr->sun_family == AF_LOCAL)
+ /* Get an address port for the desired destination address. */
+ error_t create_address_port (io_t port,
+ const struct sockaddr_un *addr,
+ socklen_t addr_len,
+ addr_port_t *aport)
{
- /* For the local domain, we must look up the name as a file and talk
- to it with the ifsock protocol. */
- file_t file = __file_name_lookup (addr->sun_path, 0, 0);
- if (file == MACH_PORT_NULL)
- return -1;
- err = __ifsock_getsockaddr (file, &aport);
- __mach_port_deallocate (__mach_task_self (), file);
- if (err == MIG_BAD_ID || err == EOPNOTSUPP)
- /* The file did not grok the ifsock protocol. */
- err = ENOTSOCK;
- if (err)
- return __hurd_fail (err);
+ error_t err_port;
+
+ if (addr->sun_family == AF_LOCAL)
+ {
+ /* For the local domain, we must look up the name as a file and talk
+ to it with the ifsock protocol. */
+ file_t file = __file_name_lookup (addr->sun_path, 0, 0);
+ if (file == MACH_PORT_NULL)
+ return errno;
+ err_port = __ifsock_getsockaddr (file, aport);
+ __mach_port_deallocate (__mach_task_self (), file);
+ if (err_port == MIG_BAD_ID || err_port == EOPNOTSUPP)
+ /* The file did not grok the ifsock protocol. */
+ err_port = ENOTSOCK;
+ }
+ else
+ {
+ err_port = __socket_create_address (port,
+ addr->sun_family,
+ (char *) addr,
+ addr_len,
+ aport);
+ }
+
+ return err_port;
}
- else
-err = EIEIO;
- /* Get an address port for the desired destination address. */
err = HURD_DPORT_USE (fd,
({
- if (err)
- err = __socket_create_address (port,
- addr->sun_family,
- (char *) addr,
- addr_len,
- &aport);
+ if (addr != NULL)
+ err = create_address_port (port, addr, addr_len,
+ &aport);
+ else
+ err = 0;
if (! err)
{
/* Send the data. */
@@ -72,12 +85,13 @@ __sendto (int fd,
NULL,
MACH_MSG_TYPE_COPY_SEND, 0,
NULL, 0, &wrote);
- __mach_port_deallocate (__mach_task_self (),
- aport);
}
err;
}));
+ if (aport != MACH_PORT_NULL)
+__mach_port_deallocate (__mach_task_self (), aport);
+
return err ? __hurd_sockfail (fd, flags, err) : wrote;
}
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] llistxattr + lremovexattr
Hi,
two simple patches to implement llistxattr and lremovexattr the same way
respectively listxattr and removexattr are implemented, just passing
O_NOLINK. This way all the *xattr functions are implemented, at least to
use the internal _hurd_xattr_*.
Thanks,
--
Pino Toscano
Hurd: provide llistxattr
Add an implementation of llistxattr based on listxattr.
2012-06-13 Pino Toscano
* sysdeps/mach/hurd/llistxattr.c: New file, copied from listxattr.c
with O_NOLINK passed to __file_name_lookup.
--- /dev/null
+++ b/sysdeps/mach/hurd/llistxattr.c
@@ -0,0 +1,36 @@
+/* Access to extended attributes on files. Hurd version.
+ Copyright (C) 2005-2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+
+#include
+#include
+#include
+#include
+#include
+
+ssize_t
+llistxattr (const char *path, char *list, size_t size)
+{
+ error_t err;
+ file_t port = __file_name_lookup (path, O_NOLINK, 0);
+ if (port == MACH_PORT_NULL)
+return -1;
+ err = _hurd_xattr_list (port, list, &size);
+ __mach_port_deallocate (__mach_task_self (), port);
+ return err ? __hurd_fail (err) : size;
+}
Hurd: provide lremovexattr
Add an implementation of lremovexattr based on removexattr.
2012-06-13 Pino Toscano
* sysdeps/mach/hurd/lremovexattr.c: New file, copied from removexattr.c
with O_NOLINK passed to __file_name_lookup.
--- /dev/null
+++ b/sysdeps/mach/hurd/lremovexattr.c
@@ -0,0 +1,36 @@
+/* Access to extended attributes on files. Hurd version.
+ Copyright (C) 2005-2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, write to the Free
+ Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ 02111-1307 USA. */
+
+#include
+#include
+#include
+#include
+#include
+
+ssize_t
+lremovexattr (const char *path, const char *name)
+{
+ error_t err;
+ file_t port = __file_name_lookup (path, O_NOLINK, 0);
+ if (port == MACH_PORT_NULL)
+return -1;
+ err = _hurd_xattr_remove (port, name);
+ __mach_port_deallocate (__mach_task_self (), port);
+ return __hurd_fail (err);
+}
signature.asc
Description: This is a digitally signed message part.
AF_LINK and sockaddr_dl
Hi,
in glibc, our bits/socket.h defines PF_LINK/AF_LINK; these seem to be
BSD feature, associated with the struct sockaddr_dl (which we do not
have), provided by .
It is not that uncommon for packages checking content of sockaddr
structs to do code like:
#ifdef AF_INET
if (sa->sa_family == AF_INET) {
struct sockaddr_in *sa_in = (struct sockaddr_in *)sa;
...
}
#endif
...
#ifdef AF_LINK
if (sa->sa_family == AF_LINK) {
struct sockaddr_dl *sa_in = (struct sockaddr_dl *)sa;
...
}
#endif
Would a patch for glibc that #if 0/comments out the PF_LINK/AF_LINK
defines be the right solution?
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] comment out PF_LINK/AF_LINK
Hi, Hurd's bits/socket.h currently define PF_LINK & AF_LINK. Although, they are usually associated with sockaddr_dl sockets, which are not implemented on Hurd, causing few build failures because of the assumption that having AF_LINK defined means there are sockaddr_dl available. The attached patch comments them out (instead of removing them, to keep their placeholders). Thanks, -- Pino Toscano Hurd: comment PF_LINK/AF_LINK defines Comment out the PF_LINK and AF_LINK defines, since they are usually associated with struct sockaddr_dl, which is not available on Hurd. 2012-06-22 Pino Toscano * sysdeps/mach/hurd/bits/socket.h (PF_LINK): Comment out. (AF_LINK): Likewise. --- a/sysdeps/mach/hurd/bits/socket.h +++ b/sysdeps/mach/hurd/bits/socket.h @@ -97,7 +97,7 @@ enum __socket_type #define PF_HYLINK 15 /* NSC Hyperchannel protocol. */ #define PF_APPLETALK 16 /* Don't use this. */ #define PF_ROUTE 17 /* Internal Routing Protocol. */ -#define PF_LINK 18 /* Link layer interface. */ +/* #define PF_LINK 18 Link layer interface. */ #define PF_XTP 19 /* eXpress Transfer Protocol (no AF). */ #define PF_COIP 20 /* Connection-oriented IP, aka ST II. */ #define PF_CNT 21 /* Computer Network Technology. */ @@ -130,7 +130,7 @@ enum __socket_type #define AF_HYLINK PF_HYLINK #define AF_APPLETALK PF_APPLETALK #define AF_ROUTE PF_ROUTE -#define AF_LINK PF_LINK +/* #define AF_LINK PF_LINK */ #define pseudo_AF_XTP PF_XTP #define AF_COIP PF_COIP #define AF_CNT PF_CNT signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] sendto: do not crash when addr is NULL
Hi, Alle mercoledì 13 giugno 2012, Pino Toscano ha scritto: > Alle martedì 12 giugno 2012, Roland McGrath ha scritto: > > That's almost right, but you should avoid calling > > __mach_port_deallocate if APORT is MACH_PORT_NULL. Also, avoid > > shadowing the local variable ERR with one of the same name in the > > subfunction. > > Hm ok, I saw other cases of shadowing by internal subroutines, and > assumed it was okay (since inside them you want to refer to the local > "err" instead of the outermost one. > > Thanks for the further review, patch attached. > (I would use the non-ChangeLog lines of the patch header as commit > log, would that be okay?) Ping, about this and the other glibc patches I sent and that are awaiting review. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] implement renameat
Hi,
the attached patch implements renameat for Hurd; it seems to work fine,
and tst-renameat now passes.
The copyright starts from the year of the rename implementation, as I
basically copied it and slightly adapted to the "at" behaviour.
Thanks,
--
Pino Toscano
Hurd: implement renameat
Provide an implementation of renameat, mostly based on rename.
2012-07-25 Pino Toscano
* sysdeps/mach/hurd/renameat.c: New file, mostly copied from rename.c.
--- /dev/null
+++ b/sysdeps/mach/hurd/renameat.c
@@ -0,0 +1,50 @@
+/* Copyright (C) 1991-2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+
+/* Rename the file OLD relative to OLDFD to NEW relative to NEWFD. */
+int
+renameat (oldfd, old, newfd, new)
+ int oldfd;
+ const char *old;
+ int newfd;
+ const char *new;
+{
+ error_t err;
+ file_t olddir, newdir;
+ const char *oldname, *newname;
+
+ olddir = __directory_name_split_at (oldfd, old, (char **) &oldname);
+ if (olddir == MACH_PORT_NULL)
+return -1;
+ newdir = __directory_name_split_at (newfd, new, (char **) &newname);
+ if (newdir == MACH_PORT_NULL)
+{
+ __mach_port_deallocate (__mach_task_self (), olddir);
+ return -1;
+}
+
+ err = __dir_rename (olddir, oldname, newdir, newname, 0);
+ __mach_port_deallocate (__mach_task_self (), olddir);
+ __mach_port_deallocate (__mach_task_self (), newdir);
+ if (err)
+return __hurd_fail (err);
+ return 0;
+}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] implement renameat
Alle mercoledì 25 luglio 2012, Roland McGrath ha scritto: > Make the top line of every new file a descriptive comment. What do you think about Renames a file using relative source and destination names. Hurd version. (of course in a single line)? > The copyright line goes second. > > Otherwise that looks fine, so go ahead and commit it with that > change. Ok, I will do. Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH] generic empty __check_native implementation
Hi,
sysdeps/posix/getaddrinfo.c calls a __check_native function to
eventually check whether an interface uses a native transport; however,
the only existing implementation of it is in
sysdeps/unix/sysv/linux/check_native.c, meaning any GNU non-Linux OS
will get an undefined reference when linking libc due to its lack.
Attached there is an empty implementation of __check_native, in the same
place as another internal inet function (__check_pf) has its generic
version. At least at a quick look it seems the empty implementation
should pose no issues (the Linux one can even not change a1_native and
a2_native).
Does it look good?
--
Pino Toscano
Provide a generic empty version of __check_native.
Add an empty implementation of __check_native, as used in the posix version of getaddrinfo.
This allows non-Linux GNU-based OSes to compile.
2012-07-26 Pino Toscano
* inet/check_native.c: New file.
--- /dev/null
+++ b/inet/check_native.c
@@ -0,0 +1,27 @@
+/* Determine whether interfaces use native transport. Generic version.
+ Copyright (C) 2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+
+
+void
+attribute_hidden
+__check_native (uint32_t a1_index, int *a1_native,
+ uint32_t a2_index, int *a2_native)
+{
+}
signature.asc
Description: This is a digitally signed message part.
[PATCH.HURD] fix fdatasync/fsync if file_sync is not supported
Hi,
Hurd's implementations of fdatasync and fsync do not take into account
the fact that file_sync could not be implemented in the receiving port,
or implemented as stub, returning (E)MIG_BAD_ID or EOPNOTSUPP.
Attached there is a patch to normalize them to EINVAL, as specified by
POSIX.
Thanks,
--
Pino Toscano
Hurd: fix fdatasync/fsync if the fd does not support file_sync
Handle the case of the fd port not implementing file_sync (returning MIG_BAD_ID) or implementing a stub (EOPNOTSUPP),
properly returning EINVAL.
2012-08-29 Pino Toscano
* sysdeps/mach/hurd/fdatasync.c: Turn ERR into EINVAL if it is
MIG_BAD_ID or EOPNOTSUPP.
* sysdeps/mach/hurd/fsync.c: Likewise.
--- a/sysdeps/mach/hurd/fdatasync.c
+++ b/sysdeps/mach/hurd/fdatasync.c
@@ -26,6 +26,10 @@ fdatasync (int fd)
{
error_t err = HURD_DPORT_USE (fd, __file_sync (port, 1, 1));
if (err)
-return __hurd_dfail (fd, err);
+{
+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ err = EINVAL;
+ return __hurd_dfail (fd, err);
+}
return 0;
}
--- a/sysdeps/mach/hurd/fsync.c
+++ b/sysdeps/mach/hurd/fsync.c
@@ -27,6 +27,10 @@ fsync (fd)
{
error_t err = HURD_DPORT_USE (fd, __file_sync (port, 1, 0));
if (err)
-return __hurd_dfail (fd, err);
+{
+ if (err == MIG_BAD_ID || err == EOPNOTSUPP)
+ err = EINVAL;
+ return __hurd_dfail (fd, err);
+}
return 0;
}
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] ptrace: use __hurd_fail for EOPNOTSUPP
Hi, attached there is a minor patch to fix the return values of unimplemented cases, i.e. using __hurd_fail to set EOPNOTSUPP properly. Thanks, -- Pino Toscano Hurd: ptrace: use __hurd_fail for EOPNOTSUPP 2012-08-29 Pino Toscano * sysdeps/mach/hurd/ptrace.c (ptrace): Use __hurd_fail to return EOPNOTSUPP. --- a/sysdeps/mach/hurd/ptrace.c +++ b/sysdeps/mach/hurd/ptrace.c @@ -160,7 +160,7 @@ ptrace (enum __ptrace_request request, . case PTRACE_SINGLESTEP: /* This is a machine-dependent kernel RPC on machines that support it. Punt. */ - return EOPNOTSUPP; + return __hurd_fail (EOPNOTSUPP); case PTRACE_ATTACH: case PTRACE_DETACH: @@ -227,7 +227,7 @@ ptrace (enum __ptrace_request request, . case PTRACE_PEEKUSER: case PTRACE_POKEUSER: /* U area, what's that? */ - return EOPNOTSUPP; + return __hurd_fail (EOPNOTSUPP); case PTRACE_GETREGS: case PTRACE_SETREGS: @@ -248,7 +248,7 @@ ptrace (enum __ptrace_request request, . return get_regs (MACHINE_THREAD_FLOAT_STATE_FLAVOR, MACHINE_THREAD_FLOAT_STATE_COUNT); #else - return EOPNOTSUPP; + return __hurd_fail (EOPNOTSUPP); #endif case PTRACE_GETFPAREGS: @@ -261,7 +261,7 @@ ptrace (enum __ptrace_request request, . return get_regs (MACHINE_THREAD_FPA_STATE_FLAVOR, MACHINE_THREAD_FPA_STATE_COUNT); #else - return EOPNOTSUPP; + return __hurd_fail (EOPNOTSUPP); #endif case PTRACE_POKETEXT: signature.asc Description: This is a digitally signed message part.
[PATCH] procfs: another fix for the process file name in stat/status
Hi,
attached there is a new patch for procfs to improve again the file name
in stat/status files for PIDs.
Basically, it just considers only the first word in case the process
name has more (e.g. when it changed its own).
Thanks,
--
Pino Toscano
From ebf2b049ea6963026766763df1697467f5806327 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Fri, 7 Sep 2012 19:14:48 +0200
Subject: [PATCH] PID stat/status: show only the first word
If a process changed its title to a multiword string, show only the first word
of it.
* process.c (args_filename_length): New function.
(process_file_gc_stat): Use args_filename_length.
(process_file_gc_status): Likewise.
---
process.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/process.c b/process.c
index 17a38ea..003bb2d 100644
--- a/process.c
+++ b/process.c
@@ -87,6 +87,14 @@ static const char *args_filename (const char *name)
return sp != NULL && *(sp + 1) != '\0' ? sp + 1 : name;
}
+static int args_filename_length (const char *name)
+{
+ const char *p = name;
+ while (*p != '\0' && *p != ' ')
+++p;
+ return p - name;
+}
+
/* Actual content generators */
static ssize_t
@@ -109,11 +117,12 @@ process_file_gc_stat (struct proc_stat *ps, char **contents)
struct procinfo *pi = proc_stat_proc_info (ps);
task_basic_info_t tbi = proc_stat_task_basic_info (ps);
thread_basic_info_t thbi = proc_stat_thread_basic_info (ps);
+ const char *fn = args_filename (proc_stat_args (ps));
/* See proc(5) for more information about the contents of each field for the
Linux procfs. */
return asprintf (contents,
- "%d (%s) %c " /* pid, command, state */
+ "%d (%.*s) %c " /* pid, command, state */
"%d %d %d " /* ppid, pgid, session */
"%d %d " /* controling tty stuff */
"%u " /* flags, as defined by */
@@ -132,7 +141,7 @@ process_file_gc_stat (struct proc_stat *ps, char **contents)
"%u %u " /* RT priority and policy */
"%llu " /* aggregated block I/O delay */
"\n",
- proc_stat_pid (ps), args_filename (proc_stat_args (ps)), state_char (ps),
+ proc_stat_pid (ps), args_filename_length (fn), fn, state_char (ps),
pi->ppid, pi->pgrp, pi->session,
0, 0, /* no such thing as a major:minor for ctty */
0, /* no such thing as CLONE_* flags on Hurd */
@@ -171,9 +180,10 @@ static ssize_t
process_file_gc_status (struct proc_stat *ps, char **contents)
{
task_basic_info_t tbi = proc_stat_task_basic_info (ps);
+ const char *fn = args_filename (proc_stat_args (ps));
return asprintf (contents,
- "Name:\t%s\n"
+ "Name:\t%.*s\n"
"State:\t%s\n"
"Tgid:\t%u\n"
"Pid:\t%u\n"
@@ -184,7 +194,7 @@ process_file_gc_status (struct proc_stat *ps, char **contents)
"VmRSS:\t%8u kB\n"
"VmHWM:\t%8u kB\n" /* ie. resident peak */
"Threads:\t%u\n",
- args_filename (proc_stat_args (ps)),
+ args_filename_length (fn), fn,
state_string (ps),
proc_stat_pid (ps), /* XXX will need more work for threads */
proc_stat_pid (ps),
--
1.7.10.4
signature.asc
Description: This is a digitally signed message part.
[PATCH] tmp: add --size to set the maximum size
Hi,
the attached patches enhance the tmpfs invocation with --size=MAX-BYTES
to set the maximum size, in addition to the current way (passing it as
first parameter).
This improves the compatibility with classic usages like putting tmpfs
entries in fstab, or mounting it with mount.
Thanks,
--
Pino Toscano
From a35bd267aba2954df85ee3403981395bf8c9e736 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Fri, 7 Sep 2012 18:21:00 +0200
Subject: [PATCH 1/2] tmpfs: extract size string parsing in an own function
* tmpfs/tmpfs.c (parse_opt_size): New function, broken out of ...
(parse_opt): ... here. Call it.
---
tmpfs/tmpfs.c | 115 -
1 file changed, 64 insertions(+), 51 deletions(-)
diff --git a/tmpfs/tmpfs.c b/tmpfs/tmpfs.c
index b72459f..2a98178 100644
--- a/tmpfs/tmpfs.c
+++ b/tmpfs/tmpfs.c
@@ -112,6 +112,66 @@ struct option_values
mode_t mode;
};
+/* Parse the size string ARG, and set *NEWSIZE with the resulting size. */
+static error_t
+parse_opt_size (const char *arg, struct argp_state *state, off_t *newsize)
+{
+ char *end = NULL;
+ intmax_t size = strtoimax (arg, &end, 0);
+ if (end == NULL || end == arg)
+{
+ argp_error (state, "argument must be a number");
+ return EINVAL;
+}
+ if (size < 0)
+{
+ argp_error (state, "negative size not meaningful");
+ return EINVAL;
+}
+ switch (*end)
+{
+ case 'g':
+ case 'G':
+ size <<= 10;
+ case 'm':
+ case 'M':
+ size <<= 10;
+ case 'k':
+ case 'K':
+ size <<= 10;
+ break;
+ case '%':
+ {
+ /* Set as a percentage of the machine's physical memory. */
+ struct vm_statistics vmstats;
+ error_t err = vm_statistics (mach_task_self (), &vmstats);
+ if (err)
+ {
+ argp_error (state, "cannot find total physical memory: %s",
+ strerror (err));
+ return err;
+ }
+ size = round_page vmstats.free_count
++ vmstats.active_count
++ vmstats.inactive_count
++ vmstats.wire_count)
+ * vm_page_size)
+ * size + 99) / 100);
+ break;
+ }
+}
+ size = (off_t) size;
+ if (size < 0)
+{
+ argp_error (state, "size too large");
+ return EINVAL;
+}
+
+ *newsize = size;
+
+ return 0;
+}
+
/* Parse a command line option. */
static error_t
parse_opt (int key, char *arg, struct argp_state *state)
@@ -166,57 +226,10 @@ parse_opt (int key, char *arg, struct argp_state *state)
}
else
{
- char *end = NULL;
- intmax_t size = strtoimax (state->argv[state->next], &end, 0);
- if (end == NULL || end == arg)
- {
- argp_error (state, "argument must be a number");
- return EINVAL;
- }
- if (size < 0)
- {
- argp_error (state, "negative size not meaningful");
- return EINVAL;
- }
- switch (*end)
- {
- case 'g':
- case 'G':
- size <<= 10;
- case 'm':
- case 'M':
- size <<= 10;
- case 'k':
- case 'K':
- size <<= 10;
- break;
- case '%':
- {
- /* Set as a percentage of the machine's physical memory. */
- struct vm_statistics vmstats;
- error_t err = vm_statistics (mach_task_self (), &vmstats);
- if (err)
- {
- argp_error (state, "cannot find total physical memory: %s",
-strerror (err));
- return err;
- }
- size = round_page vmstats.free_count
- + vmstats.active_count
- + vmstats.inactive_count
- + vmstats.wire_count)
- * vm_page_size)
-* size + 99) / 100);
- break;
- }
- }
- size = (off_t) size;
- if (size < 0)
- {
- argp_error (state, "size too large");
- return EINVAL;
- }
- values->size = size;
+ error_t err = parse_opt_size (state->argv[state->next], state,
+ &values->size);
+ if (err)
+ return err;
}
break;
--
1.7.10.4
From b733cccf1a835b91de3fbdd098e0f2fd076a4793 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Fri, 7 Sep 2012 18:24:20 +0200
Subject: [PATCH 2/2] tmpfs: add --size
Add the possibility to specify the size with the --size parameter;
this makes tmpfs more usable in fstab or Linuxish mount invocations,
since the size in such cases is a mount -o option, which gets translated
to a --foo translator argument.
The old way (specifying the size as the first argument) is left there;
although, if --size is passed then the first argument must be "tmpfs",
as it is what is passed by fstab/mount.
* tmpfs/tmpfs.c (OPT_SIZE): New macro.
(options): Add the "size" option.
(parse_opt): Use -1 to indicate when SIZE is not yet set.
: Handle case.
: Error out only when SIZE is no
Re: [PATCH] procfs: another fix for the process file name in stat/status
Alle domenica 9 settembre 2012, Samuel Thibault ha scritto:
> Pino Toscano, le Fri 07 Sep 2012 20:02:56 +0200, a écrit :
> > +static int args_filename_length (const char *name)
> > +{
> > + const char *p = name;
> > + while (*p != '\0' && *p != ' ')
> > +++p;
>
> Why not using index(name, ' ') here?
If I would use strchr, the code would be something like:
const char *p = strchr (name, ' ');
return p != NULL ? p - name : strlen (name);
which, for the common case, would mean scanning the string twice.
If it is not considered an issue I can change it anyway.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH] procfs: another fix for the process file name in stat/status
Alle lunedì 10 settembre 2012, Samuel Thibault ha scritto:
> Pino Toscano, le Mon 10 Sep 2012 17:31:32 +0200, a écrit :
> > Alle domenica 9 settembre 2012, Samuel Thibault ha scritto:
> > > Pino Toscano, le Fri 07 Sep 2012 20:02:56 +0200, a écrit :
> > > > +static int args_filename_length (const char *name)
> > > > +{
> > > > + const char *p = name;
> > > > + while (*p != '\0' && *p != ' ')
> > > > +++p;
> > >
> > > Why not using index(name, ' ') here?
> >
> > If I would use strchr, the code would be something like:
> > const char *p = strchr (name, ' ');
> > return p != NULL ? p - name : strlen (name);
> >
> > which, for the common case, would mean scanning the string twice.
>
> Ah, right. You can use strchrnul instead.
Indeed -- I didn't know it, thanks for the hint.
Updated patch attached.
--
Pino Toscano
From b376dd34c0da95ff2e63e8f91e076f306f88bbd5 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Mon, 10 Sep 2012 18:14:48 +0200
Subject: [PATCH] PID stat/status: show only the first word
If a process changed its title to a multiword string, show only the first word
of it.
* process.c (args_filename_length): New function.
(process_file_gc_stat): Use args_filename_length.
(process_file_gc_status): Likewise.
---
process.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/process.c b/process.c
index 17a38ea..68eb50a 100644
--- a/process.c
+++ b/process.c
@@ -87,6 +87,11 @@ static const char *args_filename (const char *name)
return sp != NULL && *(sp + 1) != '\0' ? sp + 1 : name;
}
+static int args_filename_length (const char *name)
+{
+ return strchrnul (name, ' ') - name;
+}
+
/* Actual content generators */
static ssize_t
@@ -109,11 +114,12 @@ process_file_gc_stat (struct proc_stat *ps, char **contents)
struct procinfo *pi = proc_stat_proc_info (ps);
task_basic_info_t tbi = proc_stat_task_basic_info (ps);
thread_basic_info_t thbi = proc_stat_thread_basic_info (ps);
+ const char *fn = args_filename (proc_stat_args (ps));
/* See proc(5) for more information about the contents of each field for the
Linux procfs. */
return asprintf (contents,
- "%d (%s) %c " /* pid, command, state */
+ "%d (%.*s) %c " /* pid, command, state */
"%d %d %d " /* ppid, pgid, session */
"%d %d " /* controling tty stuff */
"%u " /* flags, as defined by */
@@ -132,7 +138,7 @@ process_file_gc_stat (struct proc_stat *ps, char **contents)
"%u %u " /* RT priority and policy */
"%llu " /* aggregated block I/O delay */
"\n",
- proc_stat_pid (ps), args_filename (proc_stat_args (ps)), state_char (ps),
+ proc_stat_pid (ps), args_filename_length (fn), fn, state_char (ps),
pi->ppid, pi->pgrp, pi->session,
0, 0, /* no such thing as a major:minor for ctty */
0, /* no such thing as CLONE_* flags on Hurd */
@@ -171,9 +177,10 @@ static ssize_t
process_file_gc_status (struct proc_stat *ps, char **contents)
{
task_basic_info_t tbi = proc_stat_task_basic_info (ps);
+ const char *fn = args_filename (proc_stat_args (ps));
return asprintf (contents,
- "Name:\t%s\n"
+ "Name:\t%.*s\n"
"State:\t%s\n"
"Tgid:\t%u\n"
"Pid:\t%u\n"
@@ -184,7 +191,7 @@ process_file_gc_status (struct proc_stat *ps, char **contents)
"VmRSS:\t%8u kB\n"
"VmHWM:\t%8u kB\n" /* ie. resident peak */
"Threads:\t%u\n",
- args_filename (proc_stat_args (ps)),
+ args_filename_length (fn), fn,
state_string (ps),
proc_stat_pid (ps), /* XXX will need more work for threads */
proc_stat_pid (ps),
--
1.7.10.4
signature.asc
Description: This is a digitally signed message part.
[PATCH] libpthread: pthread_create: turn ENOMEM to EAGAIN
Hi, it seems that in some occasions (i.e on malloc failure) pthread_create can return ENOMEM, which is not a valid POSIX error number. Attached there is a patch for libpthread to normalize ENOMEM to EAGAIN. Thanks, -- Pino Toscano From 2ee7e1e143784452ef325a0c80c106e972a3ffdc Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sat, 15 Sep 2012 19:14:21 +0200 Subject: [PATCH] pthread_create: turn ENOMEM to EAGAIN ENOMEM can be returned if some malloc fail, but it is not a valid POSIX error number for pthread_create; thus turn it to EAGAIN. * pthread/pt-create.c (pthread_create): Turn ENOMEM to EAGAIN. --- pthread/pt-create.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/pthread/pt-create.c b/pthread/pt-create.c index 4d81a95..25a3607 100644 --- a/pthread/pt-create.c +++ b/pthread/pt-create.c @@ -66,6 +66,8 @@ pthread_create (pthread_t *thread, const pthread_attr_t *attr, err = __pthread_create_internal (&pthread, attr, start_routine, arg); if (! err) *thread = pthread->thread; + else if (err == ENOMEM) +err = EAGAIN; return err; } -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
lookup behaviour with root directory
Hi, I was investigating why fdopendir() would succeed returning a non-null DIR* for a valid fd of an open file (while it should fail with ENOTDIR). So far I traced the steps that happen: - in glibc, __fdopendir calls __file_name_lookup_under with the fd port, "/" as file name and O_DIRECTORY among the flags - __hurd_file_name_lookup strips leading "/" from the file name but adding trailing "/" if O_DIRECTORY is set (to force the lookup as directory), so the actual lookup RPC invoked (which is dir_lookup, since 0 is passed to __hurd_file_name_lookup) has "/" as file name - on the hurd side: diskfs_S_dir_lookup (or netfs_S_dir_lookup, since both behave the same in what follows) first strips leading "/", leaving the path as empty string, which causes the "fast path" if at line 67 to be executed, picking as result node the one associated to the fd port we had; below, at the gotit label, the type check based on the stat of the node does not trigger the if returning ENOTDIR, because mustbedir is false - back on the glibc side: the dir_lookup succeeded so the lookup called in __fdopendir passes and a new DIR* is returned At this point I'm a bit puzzled as to where should be the right place to fix, even because I don't know much the parts involved. It seems the lookup logic (implemented in the lookup glibc functions and in e.g. diskfs_S_dir_lookup) to force a directory based on trailing "/" works for paths with at least one component (like "/foo/"), so a solution in libdiskfs/libnetfs could be make the "fast path" set mustbedir if the flags contain O_DIRECTORY, although I'm not sure whether that would break assumptions done in other places. Otherwise, I'm not yet sure how fix the lookup glibc functions. As I said, I'm pretty new to the involved parts (lookup stuff), so if you have more ideas I will hear them. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: qoth 2012 q1/q2, preliminary
Hi, Alle lunedì 17 settembre 2012, Arne Babenhauserheide ha scritto: > I finally got around to drafting a qoth the week before last week. > > I only checked bug-hurd for that, so it would be great if you could > have a look at it and add any info I missed. Thanks for your work, few notes below. > * Richard Braun [libpcap](http://lists.gnu.org/archive/html/bug- > hurd/2012-01/msg00059.html) bringing wireshark and [pcap_inject] > (http://lists.gnu.org/archive/html/bug-hurd/2012-03/msg0.html) > for easier network testing. It looks like it misses some text? :) > * Thomas Schwinge [moved](http://lists.gnu.org/archive/html/bug- > hurd/2012-03/msg00063.html) the translators [cvsfs] > (http://git.savannah.gnu.org/cgit/hurd/incubator.git/log/?h=cvsfs/mas > ter), [libfuse] > (http://git.savannah.gnu.org/cgit/hurd/incubator.git/log/?h=libfuse/m > aster) and [smbfs] > (http://git.savannah.gnu.org/cgit/hurd/incubator.git/log/?h=smbfs/mas > ter) into the incubator git repository, reducing the barrier of entry > to improving them, so integrating cvs and samba in the filesystem > and using FUSE translators can be stabilized more easily. Technically libfuse is not a translator, but a library. Also, it is not uptodate to the latest libfuse API, so that with the fact that the majority of FUSE file systems use pthreads made it not that useful, yet. > * Ludovic Courtes [fixed invalid port deallocation in `symlink'] > (http://lists.gnu.org/archive/html/bug-hurd/2012-03/msg00013.html) > and [made console-run resilient against missing /dev/console] > (http://lists.gnu.org/archive/html/bug-hurd/2012-03/msg2.html), > improving the overall reliability of the system. Not sure how they "improve the overall reliability of the system"... > * Pino Toscano improved the POSIX compliance of the Hurd [for > nanosleep] > (http://lists.gnu.org/archive/html/bug-hurd/2012-04/msg00130.html) > [ptsname_r] > (http://lists.gnu.org/archive/html/bug-hurd/2012-04/msg00122.html), > [getlogin_r](http://lists.gnu.org/archive/html/bug- > hurd/2012-04/msg00121.html), [getgroups] > (http://lists.gnu.org/archive/html/bug-hurd/2012-04/msg00120.html) > and > [sendto](http://lists.gnu.org/archive/html/bug-hurd/2012-06/msg9 > .html), making it easier to port POSIX programs. They don't simplify any porting at all, they just fix POSIX incompatibilities and bugs. Also, the ptsname_r patch is not merged, yet. > * Thomas DiModica [merged](http://lists.gnu.org/archive/html/bug- > hurd/2012-06/msg00018.html) the cthreads to pthreads patch to Hurd > master "merging a patch to master" sounds like as now the master branch of hurd.git has that patch (which is not the case). > * Samuel Thibault [made iconx > build](http://lists.gnu.org/archive/html/bug- > hurd/2012-06/msg4.html), which fullfills a requirement for > FTBFS. What should "fullfills a requirement for FTBFS" mean? -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH] gnumach: configure: add --with-version-suffix=STRING
Hi, the attached gnumach patch allows to specify a custom suffix for the gnumach version string. This can be useful to distinguish different kernels, for example. (A much simplier version of this patch has been developed by Samuel Thibault, and it's part of the Debian package for a couple of months.) Thanks, -- Pino Toscano From 399f9112f763c095abe44567135307c554746e9e Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sun, 23 Sep 2012 23:53:00 +0200 Subject: [PATCH] configure: add --with-version-suffix=STRING Add the possibility to append a custom string to the gnumach version string. (Useful to distinguish different flavours of kernel.) * configure.ac (--with-version-suffix): New option. (PACKAGE_VERSION_SUFFIX): New variable, set with the value of --with-version-suffix. * version.c.in (version): Append PACKAGE_VERSION_SUFFIX. --- configure.ac |8 version.c.in |2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 3a7d3be..da5e09e 100644 --- a/configure.ac +++ b/configure.ac @@ -101,6 +101,14 @@ AC_CHECK_PROG([PATCH], [patch], [patch], [patch-not-found]) # configure fragments. # +# An optional suffix for the version string. +AC_MSG_CHECKING([for the suffix for the version string]) +AC_ARG_WITH(version-suffix, AC_HELP_STRING([--with-version-suffix=STRING], +[append STRING to the version string]), +[PACKAGE_VERSION_SUFFIX=$withval]) +AC_MSG_RESULT([$PACKAGE_VERSION_SUFFIX]) +AC_SUBST([PACKAGE_VERSION_SUFFIX]) + # The test suite. m4_include([tests/configfrag.ac]) diff --git a/version.c.in b/version.c.in index d894d7f..2c57314 100644 --- a/version.c.in +++ b/version.c.in @@ -1,2 +1,2 @@ /* @configure_input@ */ -const char version[] = "@PACKAGE_NAME@ @PACKAGE_VERSION@"; +const char version[] = "@PACKAGE_NAME@ @PACKAGE_VERSION@@PACKAGE_VERSION_SUFFIX@"; -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] gnumach: configure: add --with-version-suffix=STRING
Hi, Alle lunedì 24 settembre 2012, Thomas Schwinge ha scritto: > Generally OK, but two comments: > > On Mon, 24 Sep 2012 00:04:20 +0200, Pino Toscano wrote: > > Subject: [PATCH] configure: add --with-version-suffix=STRING > > > > Add the possibility to append a custom string to the gnumach > > version string. (Useful to distinguish different flavours of > > kernel.) > > > > * configure.ac (--with-version-suffix): New option. > > (PACKAGE_VERSION_SUFFIX): New variable, set with the value of > > --with-version-suffix. > > * version.c.in (version): Append PACKAGE_VERSION_SUFFIX. > > --- > > > > configure.ac |8 > > version.c.in |2 +- > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 3a7d3be..da5e09e 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -101,6 +101,14 @@ AC_CHECK_PROG([PATCH], [patch], [patch], > > [patch-not-found]) > > > > # configure fragments. > > # > > > > +# An optional suffix for the version string. > > +AC_MSG_CHECKING([for the suffix for the version string]) > > +AC_ARG_WITH(version-suffix, > > AC_HELP_STRING([--with-version-suffix=STRING], + > > [append STRING to the version string]), > > +[PACKAGE_VERSION_SUFFIX=$withval]) > > +AC_MSG_RESULT([$PACKAGE_VERSION_SUFFIX]) > > +AC_SUBST([PACKAGE_VERSION_SUFFIX]) > > Does the AC_MSG_CHECKING and AC_MSG_RESULT add any useful information > to the build log? (You should know which option you're configuring > with.) IMHO it makes the log sligtly more useful, since that option somehow modifies gnumach so it could be useful to know which options affected its build; also a log could be provided by other people, so the more information about its build it provides, the better. That said, I'm not overly advocating it, so if it is not deemed worth, no problem. > Is --with-version-suffix the standard name for this option? No idea, to be honest :) -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] mknod: allow to create also sockets
Hi,
attached there is a small patch to allow Hurd's mknod(at) to create also
sockets; while POSIX does not specify them, on Linux they seem to be
supported, and the implementation is very small.
I also took the liberty of simplifying the checks done to decide whether
use the device number provided.
Thanks,
--
Pino Toscano
Hurd: mknodat: create also sockets
2012-09-28 Pino Toscano
* sysdeps/mach/hurd/xmknodat.c: Allow to create sockets. Simplify the
check for translators needing the device number.
--- a/sysdeps/mach/hurd/xmknodat.c
+++ b/sysdeps/mach/hurd/xmknodat.c
@@ -39,6 +39,7 @@ __xmknodat (int vers, int fd, const char
char buf[100], *bp;
const char *translator;
size_t len;
+ int needdevice = 0;
if (vers != _MKNOD_VER)
return __hurd_fail (EINVAL);
@@ -47,17 +48,24 @@ __xmknodat (int vers, int fd, const char
{
translator = _HURD_CHRDEV;
len = sizeof (_HURD_CHRDEV);
+ needdevice = 1;
}
else if (S_ISBLK (mode))
{
translator = _HURD_BLKDEV;
len = sizeof (_HURD_BLKDEV);
+ needdevice = 1;
}
else if (S_ISFIFO (mode))
{
translator = _HURD_FIFO;
len = sizeof (_HURD_FIFO);
}
+ else if (S_ISSOCK (mode))
+{
+ translator = _HURD_IFSOCK;
+ len = sizeof (_HURD_IFSOCK);
+}
else if (S_ISREG (mode))
{
translator = NULL;
@@ -69,7 +77,7 @@ __xmknodat (int vers, int fd, const char
return -1;
}
- if (translator != NULL && ! S_ISFIFO (mode))
+ if (needdevice)
{
/* We set the translator to "ifmt\0major\0minor\0", where IFMT
depends on the S_IFMT bits of our MODE argument, and MAJOR and
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] mknod: allow to create also sockets
Alle venerdì 28 settembre 2012, Roland McGrath ha scritto: > What's the point of ever creating a local-domain socket node with > mknod? To be honest, I was making tst-mknodat.c pass on Hurd; if it does not make sense that mknod can create sockets, then that test should be removed, since all it does is testing the creation of a single socket. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] mknod: allow to create also sockets
Alle venerdì 28 settembre 2012, Roland McGrath ha scritto:
> > To be honest, I was making tst-mknodat.c pass on Hurd; if it does
> > not make sense that mknod can create sockets, then that test
> > should be removed, since all it does is testing the creation of a
> > single socket.
>
> I think the intent of the change is to test the mknodat interface,
> not any particular kind of node. Just change the test to create a
> FIFO instead.
Right, patch attached.
--
Pino Toscano
tst-mknodat: create a FIFO instead of a socket
A FIFO is the only special file which is guaranteed to be created with mknod/mknodat.
2012-09-28 Pino Toscano
* io/tst-mknodat.c: Create a FIFO instead of a socket.
--- a/io/tst-mknodat.c
+++ b/io/tst-mknodat.c
@@ -80,8 +80,8 @@ do_test (void)
}
closedir (dir);
- /* Create a new directory. */
- int e = mknodat (dir_fd, "some-sock", 0777 | S_IFSOCK, 0);
+ /* Create a new fifo. */
+ int e = mknodat (dir_fd, "some-fifo", 0777 | S_IFIFO, 0);
if (e == -1)
{
if (errno == ENOSYS)
@@ -90,19 +90,19 @@ do_test (void)
return 0;
}
- puts ("socket creation failed");
+ puts ("fifo creation failed");
return 1;
}
struct stat64 st1;
- if (fstatat64 (dir_fd, "some-sock", &st1, 0) != 0)
+ if (fstatat64 (dir_fd, "some-fifo", &st1, 0) != 0)
{
puts ("fstat64 failed");
return 1;
}
- if (!S_ISSOCK (st1.st_mode))
+ if (!S_ISFIFO (st1.st_mode))
{
- puts ("mknodat did not create a Unix domain socket");
+ puts ("mknodat did not create a fifo");
return 1;
}
@@ -124,15 +124,15 @@ do_test (void)
puts ("2nd fdopendir failed");
return 1;
}
- bool has_some_sock = false;
+ bool has_some_fifo = false;
while ((d = readdir64 (dir)) != NULL)
-if (strcmp (d->d_name, "some-sock") == 0)
+if (strcmp (d->d_name, "some-fifo") == 0)
{
- has_some_sock = true;
+ has_some_fifo = true;
#ifdef _DIRENT_HAVE_D_TYPE
- if (d->d_type != DT_UNKNOWN && d->d_type != DT_SOCK)
+ if (d->d_type != DT_UNKNOWN && d->d_type != DT_FIFO)
{
- puts ("d_type for some-sock wrong");
+ puts ("d_type for some-fifo wrong");
return 1;
}
#endif
@@ -144,13 +144,13 @@ do_test (void)
}
closedir (dir);
- if (!has_some_sock)
+ if (!has_some_fifo)
{
- puts ("some-sock not in directory list");
+ puts ("some-fifo not in directory list");
return 1;
}
- if (unlinkat (dir_fd, "some-sock", 0) != 0)
+ if (unlinkat (dir_fd, "some-fifo", 0) != 0)
{
puts ("unlinkat failed");
return 1;
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] mknod: allow to create also sockets
Alle venerdì 28 settembre 2012, Roland McGrath ha scritto: > That change looks fine if the test still passes on Linux. It does on Linux/x86_64, so I pushed it. Thanks for the reviews, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH] diskfs/pathconf: handle _PC_2_SYMLINKS
Hi, attached there is a simple patch to make (f)pathconf(path, _PC_2_SYMLINKS) working for libdiskfs-based translators, properly indicating whether the FS supports symlinks. Thanks, -- Pino Toscano From edbb0cc20709e0599e1490b81aa1841114f3f0bc Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sun, 30 Sep 2012 14:15:27 +0200 Subject: [PATCH] libdiskfs: handle _PC_2_SYMLINKS in pathconf Advertize the possibility to handle symlinks depending on DISKFS_SHORTCUT_SYMLINK. * libdiskfs/io-pathconf.c (diskfs_S_io_pathconf): Handle _PC_2_SYMLINKS too. --- libdiskfs/io-pathconf.c |4 1 file changed, 4 insertions(+) diff --git a/libdiskfs/io-pathconf.c b/libdiskfs/io-pathconf.c index 38e277c..bb192fe 100644 --- a/libdiskfs/io-pathconf.c +++ b/libdiskfs/io-pathconf.c @@ -72,6 +72,10 @@ diskfs_S_io_pathconf (struct protid *cred, *value = 32; break; +case _PC_2_SYMLINKS: + *value = diskfs_shortcut_symlink ? 1 : 0; + break; + default: return EINVAL; } -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] getconf: fix the value of _CS_PATH
Hi, attached there is a patch to add a custom version (copied from the unix sysdep) of confstr.h, to avoid using the bsd header which adds a non- existing /usr/ucb. Thanks, -- Pino Toscano Hurd: fix the _CS_PATH getconf value Copy sysdeps/unix/confstr.h to avoid using sysdeps/unix/bsd/confstr.h which adds a non-existing /usr/ucb. 2012-08-30 Pino Toscano * sysdeps/mach/hurd/confstr.h: New file, copied from sysdeps/unix/confstr.h. --- /dev/null +++ b/sysdeps/mach/hurd/confstr.h @@ -0,0 +1 @@ +#define CS_PATH "/bin:/usr/bin" signature.asc Description: This is a digitally signed message part.
[PATCH] libpthread: make use of the "pthread" sysdep
Hi, The following two patches allow to a) make use of the "pthread" glibc sysdep, which provides implementation for some functions using pthreads (e.g. aio stuff) b) hook the libpthread version inside glibc, so it is properly returned by confstr. -- Pino Toscano From 4335cb271c7ebf80e90bcaf6cfeb6e63adef62ab Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sun, 30 Sep 2012 23:40:47 +0200 Subject: [PATCH 1/2] Require the "pthread" sysdep This allows the use of glibc sysdeps that use pthread-based implementations. * sysdeps/mach/hurd/Implies: Add pthread. --- sysdeps/mach/hurd/Implies |1 + 1 file changed, 1 insertion(+) diff --git a/sysdeps/mach/hurd/Implies b/sysdeps/mach/hurd/Implies index 16b8348..c32378a 100644 --- a/sysdeps/mach/hurd/Implies +++ b/sysdeps/mach/hurd/Implies @@ -1 +1,2 @@ hurd +pthread -- 1.7.10.4 From 24b10a76fd1f89ad9c0623fe4b19f2557c6dc656 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sun, 30 Sep 2012 23:44:24 +0200 Subject: [PATCH 2/2] New Makefile snippet for the "pthread" sysdep Hook the libpthread version when compiling glibc's confstr.c, so that confstr can show it. * sysdeps/pthread/Makefile: New file. --- sysdeps/pthread/Makefile |3 +++ 1 file changed, 3 insertions(+) create mode 100644 sysdeps/pthread/Makefile diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile new file mode 100644 index 000..1c30542 --- /dev/null +++ b/sysdeps/pthread/Makefile @@ -0,0 +1,3 @@ +ifeq ($(subdir),posix) +CFLAGS-confstr.c += -DLIBPTHREAD_VERSION='"libpthread $(pthread-version)"' +endif -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] gnumach: configure: add --with-version-suffix=STRING
Hi, Alle giovedì 27 settembre 2012, Thomas Schwinge ha scritto: > On Mon, 24 Sep 2012 02:04:05 +0200, Guillem Jover wrote: > > On Mon, 2012-09-24 at 00:30:39 +0200, Pino Toscano wrote: > > > Alle lunedì 24 settembre 2012, Thomas Schwinge ha scritto: > > > > On Mon, 24 Sep 2012 00:04:20 +0200, Pino Toscano wrote: > > > > > +# An optional suffix for the version string. > > > > > +AC_MSG_CHECKING([for the suffix for the version string]) > > > > > +AC_ARG_WITH(version-suffix, > > > > > AC_HELP_STRING([--with-version-suffix=STRING], + > > > > > [append STRING to the version string]), > > > > > +[PACKAGE_VERSION_SUFFIX=$withval]) > > > > > +AC_MSG_RESULT([$PACKAGE_VERSION_SUFFIX]) > > > > > +AC_SUBST([PACKAGE_VERSION_SUFFIX]) > > > > > > > > Does the AC_MSG_CHECKING and AC_MSG_RESULT add any useful > > > > information to the build log? (You should know which option > > > > you're configuring with.) > > > > Maybe use AC_MSG_NOTICE instead? > > > > > IMHO it makes the log sligtly more useful, since that option > > > somehow modifies gnumach so it could be useful to know which > > > options affected its build; also a log could be provided by > > > other people, so the more information about its build it > > > provides, the better. > > > > Yeah, I agree. > > I'm not aware of configure scripts typically doing that? I'd suggest > to drop that entirely, but don't insist on that. Ok, I've decided to drop that for now. > > > > Is --with-version-suffix the standard name for this option? > > > > > > No idea, to be honest :) > > > > I don't think there's any such standard name for this. > > Right, probably not too wide-spread. GCC has: > > --with-pkgversion=PKG Use PKG in the version string in place > of "GCC" > > Without setting that: > > i686-pc-gnu-gcc (GCC) 4.7.0 20110905 (experimental) > > With setting it: > > gcc (Debian 4.7.1-7) 4.7.1 > > (Search for case-insensitive pkgversion in config/acx.m4, > gcc/Makefile.in, gcc/toplev.c, gcc/version.c.) Would that seem > reasonable for GNU Mach, too? It is not the same thing, --with-pkgversion replaces only the part inside the brackets, but it does not affect the actual version. I updated the patch as said above. -- Pino Toscano From 81d62b666ac10f379196948a99af321f48171b6b Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Mon, 1 Oct 2012 19:57:05 +0200 Subject: [PATCH] configure: add --with-version-suffix=STRING Add the possibility to append a custom string to the gnumach version string. (Useful to distinguish different flavours of kernel.) * configure.ac (--with-version-suffix): New option. (PACKAGE_VERSION_SUFFIX): New variable, set with the value of --with-version-suffix. * version.c.in (version): Append PACKAGE_VERSION_SUFFIX. --- configure.ac |6 ++ version.c.in |2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 3a7d3be..368a29a 100644 --- a/configure.ac +++ b/configure.ac @@ -101,6 +101,12 @@ AC_CHECK_PROG([PATCH], [patch], [patch], [patch-not-found]) # configure fragments. # +# An optional suffix for the version string. +AC_ARG_WITH(version-suffix, AC_HELP_STRING([--with-version-suffix=STRING], +[string to append to the version string]), +[PACKAGE_VERSION_SUFFIX=$withval]) +AC_SUBST([PACKAGE_VERSION_SUFFIX]) + # The test suite. m4_include([tests/configfrag.ac]) diff --git a/version.c.in b/version.c.in index d894d7f..2c57314 100644 --- a/version.c.in +++ b/version.c.in @@ -1,2 +1,2 @@ /* @configure_input@ */ -const char version[] = "@PACKAGE_NAME@ @PACKAGE_VERSION@"; +const char version[] = "@PACKAGE_NAME@ @PACKAGE_VERSION@@PACKAGE_VERSION_SUFFIX@"; -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] gnumach: configure: add --with-version-suffix=STRING
Alle martedì 2 ottobre 2012, Thomas Schwinge ha scritto: > Hi! > > On Mon, 1 Oct 2012 19:59:35 +0200, Pino Toscano wrote: > > Alle giovedì 27 settembre 2012, Thomas Schwinge ha scritto: > > > On Mon, 24 Sep 2012 02:04:05 +0200, Guillem Jover > > > > wrote: > > > > On Mon, 2012-09-24 at 00:30:39 +0200, Pino Toscano wrote: > > > > > Alle lunedì 24 settembre 2012, Thomas Schwinge ha scritto: > > > > > > Is --with-version-suffix the standard name for this option? > > > > > > > > > > No idea, to be honest :) > > > > > > > > I don't think there's any such standard name for this. > > > > > > Right, probably not too wide-spread. GCC has: > > > --with-pkgversion=PKG Use PKG in the version string in > > > place > > > > > > of "GCC" > > > > > > Without setting that: > > > i686-pc-gnu-gcc (GCC) 4.7.0 20110905 (experimental) > > > > > > With setting it: > > > gcc (Debian 4.7.1-7) 4.7.1 > > > > > > (Search for case-insensitive pkgversion in config/acx.m4, > > > gcc/Makefile.in, gcc/toplev.c, gcc/version.c.) Would that seem > > > reasonable for GNU Mach, too? > > > > It is not the same thing, --with-pkgversion replaces only the part > > inside the brackets, but it does not affect the actual version. > > Well, my implied question was whether that might be deemed > appropritate for GNU Mach, too: »gnumach (Debian > 1.3.99.dfsg.git20110227-1) 1.3.99«. I see, so you are suggesting to modify the default version string to be something like: | gnumach (GNU Mach) 1.3.99 with --with-pkgversion affecting the part in brackets, so --with-pkgversion="My own, #1" would produce | gnumach (My own, #1) 1.3.99 ? If so, then what should be the prefix and the default pkgversion to use? -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] getconf: fix the value of _CS_PATH
Hi, Alle lunedì 1 ottobre 2012, Andreas Jaeger ha scritto: > On Sunday, September 30, 2012 17:53:33 Pino Toscano wrote: > > attached there is a patch to add a custom version (copied from the > > unix sysdep) of confstr.h, to avoid using the bsd header which adds > > a non- existing /usr/ucb. > > An alternative would be to use an include like: > > /* We do not want the sysdeps/unix/bsd version. */ > #include Good idea, thanks for the hint. Updated patch attached. -- Pino Toscano Hurd: fix the _CS_PATH getconf value Create a new Hurd-specific confstr.h including sysdeps/unix/confstr.h directly to skip sysdeps/unix/bsd/confstr.h (which adds a non-existing /usr/ucb). 2012-10-03 Pino Toscano * sysdeps/mach/hurd/confstr.h: New file. --- /dev/null +++ b/sysdeps/mach/hurd/confstr.h @@ -0,0 +1,2 @@ +/* We do not want the sysdeps/unix/bsd version. */ +#include signature.asc Description: This is a digitally signed message part.
Re: [PATCH] gnumach: configure: add --with-version-suffix=STRING
Alle mercoledì 3 ottobre 2012, Thomas Schwinge ha scritto: > Hi! > > On Wed, 3 Oct 2012 17:50:38 +0200, Pino Toscano wrote: > > Alle martedì 2 ottobre 2012, Thomas Schwinge ha scritto: > > > On Mon, 1 Oct 2012 19:59:35 +0200, Pino Toscano > > > > wrote: > > > > Alle giovedì 27 settembre 2012, Thomas Schwinge ha scritto: > > > > > --with-pkgversion=PKG Use PKG in the version string > > > > > in place > > > > > > > > > > of "GCC" > > > > > > > > > > Without setting that: > > > > > i686-pc-gnu-gcc (GCC) 4.7.0 20110905 (experimental) > > > > > > > > > > With setting it: > > > > > gcc (Debian 4.7.1-7) 4.7.1 > > > > > > > > > > (Search for case-insensitive pkgversion in config/acx.m4, > > > > > gcc/Makefile.in, gcc/toplev.c, gcc/version.c.) Would that > > > > > seem reasonable for GNU Mach, too? > > > > I see, so you are suggesting to modify the default version string > > to be > > > > something like: > > | gnumach (GNU Mach) 1.3.99 > > > > with --with-pkgversion affecting the part in brackets, so > > --with-pkgversion="My own, #1" would produce > > > > | gnumach (My own, #1) 1.3.99 > > Yes. But if you don't think that's worth it/don't feel like working > on that, please just commit your previous patch. > > > ? If so, then what should be the prefix and the default pkgversion > > to use? > > I think what you suggested about looks fine. Ok, new version attached. -- Pino Toscano From 2bc7dc59318264264862d080641874be423e3407 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Thu, 4 Oct 2012 18:10:36 +0200 Subject: [PATCH] Change the version string Switch the default version string to "gnumach (PKGVERSION) VERSION", with PKGVERSION being "GNU Mach" (i.e. the package name) by default, and overridable by the user with --with-pkgversion=PKG on configure. * configure.ac (--with-pkgversion): New option. (PKGVERSION, PACKAGE_SHORTNAME): New variables. * version.c.in (version): Change it to "SHORTNAME (PKGVERSION) VERSION". * version.m4 (AC_PACKAGE_SHORTNAME): New define. --- configure.ac | 10 ++ version.c.in |2 +- version.m4 |1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 3a7d3be..b8388ec 100644 --- a/configure.ac +++ b/configure.ac @@ -101,6 +101,16 @@ AC_CHECK_PROG([PATCH], [patch], [patch], [patch-not-found]) # configure fragments. # +# Configure the pkgversion string. +AC_ARG_WITH([pkgversion], + AS_HELP_STRING([--with-pkgversion=PKG], + [Use PKG in the version string in place of "AC_PACKAGE_NAME"]), + [PKGVERSION="$withval"], + [PKGVERSION=$PACKAGE_NAME] +) +AC_SUBST([PACKAGE_SHORTNAME], [AC_PACKAGE_SHORTNAME]) +AC_SUBST([PKGVERSION]) + # The test suite. m4_include([tests/configfrag.ac]) diff --git a/version.c.in b/version.c.in index d894d7f..f115e11 100644 --- a/version.c.in +++ b/version.c.in @@ -1,2 +1,2 @@ /* @configure_input@ */ -const char version[] = "@PACKAGE_NAME@ @PACKAGE_VERSION@"; +const char version[] = "@PACKAGE_SHORTNAME@ (@PKGVERSION@) @PACKAGE_VERSION@"; diff --git a/version.m4 b/version.m4 index 3bf4275..47df223 100644 --- a/version.m4 +++ b/version.m4 @@ -2,3 +2,4 @@ m4_define([AC_PACKAGE_NAME],[GNU Mach]) m4_define([AC_PACKAGE_VERSION],[1.3.99]) m4_define([AC_PACKAGE_BUGREPORT],[bug-hurd@gnu.org]) m4_define([AC_PACKAGE_TARNAME],[gnumach]) +m4_define([AC_PACKAGE_SHORTNAME],[gnumach]) -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] diskfs/pathconf: handle _PC_2_SYMLINKS
Alle lunedì 1 ottobre 2012, Roland McGrath ha scritto: > That doesn't look right to me. diskfs_shortcut_symlink means that > the filesystem implementation has its own way to store symlinks. A > Hurd filesystem supports symlinks either if there is a first-class > storage mechanism like that, or if the filesystems supports storing > passive translator records generally. Interesting, I think I got confused by the fact that all the diskfs- based filesystems in hurd currently with diskfs_shortcut_symlink set do support symlinks, and those where that variable is not set (only fatfs, IIRC) do not support symlinks. I've reverted the commit, and going to propose a better (I hope) solution for this. Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
RFC: filesystem-specific pathconf replies
Hi,
there are filesystem-specific options (e.g. _PC_2_SYMLINKS) which cannot
expressed currently with variables of libdiskfs (that is, which
libdiskfs does not know).
My proposal is to add a new function, optionally overridable by
filesystems-specific implementation, to return custom pathconf replies,
eventually overriding those already returned by the diskfs/netfs
io_pathconf.
Attached there is a prototype, without comments nor copyright; of couse
a final version would also cover libnetfs, and more filesystems
currently in hurd.git.
AFAICT, this solution should maintain source and binary compatibility,
and a filesystem providing such diskfs_pathconf implementation should
run fine even with an older libdisks (but I did not test this case).
What do you think about this solution?
--
Pino Toscano
--- a/libdiskfs/diskfs.h
+++ b/libdiskfs/diskfs.h
@@ -553,6 +553,9 @@
DISKFS_READONLY true. */
error_t diskfs_node_reload (struct node *node);
+/* The user may define this function. */
+error_t diskfs_pathconf (struct node *np, int name, int *value);
+
/* If this function is nonzero (and diskfs_shortcut_symlink is set) it
is called to set a symlink. If it returns EINVAL or isn't set,
then the normal method (writing the contents into the file data) is
--- /dev/null
+++ b/libdiskfs/pathconf.c
@@ -0,0 +1,8 @@
+#include "priv.h"
+
+error_t
+diskfs_pathconf (struct node *np, int name, int *value)
+{
+ return EINVAL;
+}
+
--- a/libdiskfs/io-pathconf.c
+++ b/libdiskfs/io-pathconf.c
@@ -30,6 +30,10 @@
if (!cred)
return EOPNOTSUPP;
+ error_t err = diskfs_pathconf (cred->po->np, name, value);
+ if (err != EINVAL)
+return err;
+
switch (name)
{
case _PC_LINK_MAX:
--- a/tmpfs/tmpfs.c
+++ b/tmpfs/tmpfs.c
@@ -98,6 +98,22 @@
return 0;
}
+error_t
+diskfs_pathconf (struct node *np, int name, int *value)
+{
+ switch (name)
+{
+case _PC_2_SYMLINKS:
+ *value = 1;
+ break;
+
+default:
+ return EINVAL;
+}
+
+ return 0;
+}
+
int diskfs_synchronous = 0;
static const struct argp_option options[] =
--- a/libdiskfs/Makefile
+++ b/libdiskfs/Makefile
@@ -51,7 +51,7 @@
remount.c console.c disk-pager.c \
name-cache.c direnter.c dirrewrite.c dirremove.c lookup.c dead-name.c \
validate-mode.c validate-group.c validate-author.c validate-flags.c \
- validate-rdev.c validate-owner.c extra-version.c
+ validate-rdev.c validate-owner.c extra-version.c pathconf.c
SRCS = $(OTHERSRCS) $(FSSRCS) $(IOSRCS) $(FSYSSRCS) $(IFSOCKSRCS)
installhdrs = diskfs.h diskfs-pager.h
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH.HURD] fix fdatasync/fsync if file_sync is not supported
Alle mercoledì 29 agosto 2012, Pino Toscano ha scritto: > Hurd's implementations of fdatasync and fsync do not take into > account the fact that file_sync could not be implemented in the > receiving port, or implemented as stub, returning (E)MIG_BAD_ID or > EOPNOTSUPP. Attached there is a patch to normalize them to EINVAL, > as specified by POSIX. Ping. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] ptrace: use __hurd_fail for EOPNOTSUPP
Alle mercoledì 29 agosto 2012, Pino Toscano ha scritto: > attached there is a minor patch to fix the return values of > unimplemented cases, i.e. using __hurd_fail to set EOPNOTSUPP > properly. Ping. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH.HURD] fix fdatasync/fsync if file_sync is not supported
Alle venerdì 26 ottobre 2012, Samuel Thibault ha scritto:
> Svante Signell, le Fri 26 Oct 2012 08:03:43 +0200, a écrit :
> > On Thu, 2012-10-25 at 19:47 +0200, Samuel Thibault wrote:
> > > Roland McGrath, le Thu 25 Oct 2012 09:49:51 -0700, a écrit :
> > > > We don't generally handle MIG_BAD_ID. That error indicates a
> > > > server not implementing the protocol for which it's being
> > > > used, which is a server bug.
> > >
> > > Pino, in which case did you actually get MIG_BAD_ID precisely?
with sample.c:
int main()
{
fsync(STDOUT_FILENO)
}
$ ./sample | less
> (even if it's probably not right in going crazy if it doesn't return
> EINVAL).
Well, one could note throwing an error value which is not even a POSIX
errno does not sound correct to send to userland.
> In the pipe case (i.e. pflocal), libtrivfs is used, EOPNOTSUPP is
> returned, right?
The run above would have fsync() set errno = -303, MIG_BAD_ID.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH.HURD] fix fdatasync/fsync if file_sync is not supported
Alle giovedì 25 ottobre 2012, Roland McGrath ha scritto:
> We don't generally handle MIG_BAD_ID. That error indicates a server
> not implementing the protocol for which it's being used, which is a
> server bug. If anything, we'd translate that to EGRATUITOUS in the
> general case, and perhaps to EOPNOTSUPP in particular cases where
> it's not an ironclad part of the interface contract that it
> implement the particular protocol. So I think we should separate
> the MIG_BAD_ID case and treat it more generally, if at all.
OK, it seems it could actually be a bug in pflocal, sorry for the bad
investigation of thi.s
> Since it's the norm for servers to return EOPNOTSUPP for
> miscellaneous RPCs they aren't implementing, translating that to
> EINVAL for fsync/fdatasync is reasonable. It should have a comment
> saying that POSIX.1 specifies EINVAL for a file descriptor on which
> sync is not supported.
Attached an updated patch to handle only EOPNOTSUPP, with comment added.
Thanks,
--
Pino Toscano
Hurd: fix fdatasync/fsync if the fd does not support file_sync
Handle the case of the fd port implementing a stub (EOPNOTSUPP),
properly returning EINVAL.
2012-10-29 Pino Toscano
* sysdeps/mach/hurd/fdatasync.c: Turn ERR into EINVAL if it is
EOPNOTSUPP.
* sysdeps/mach/hurd/fsync.c: Likewise.
--- a/sysdeps/mach/hurd/fdatasync.c
+++ b/sysdeps/mach/hurd/fdatasync.c
@@ -26,6 +26,12 @@ fdatasync (int fd)
{
error_t err = HURD_DPORT_USE (fd, __file_sync (port, 1, 1));
if (err)
-return __hurd_dfail (fd, err);
+{
+ if (err == EOPNOTSUPP)
+ /* If the file descriptor does not support sync, return EINVAL
+ as POSIX specifies. */
+ err = EINVAL;
+ return __hurd_dfail (fd, err);
+}
return 0;
}
--- a/sysdeps/mach/hurd/fsync.c
+++ b/sysdeps/mach/hurd/fsync.c
@@ -27,6 +27,12 @@ fsync (fd)
{
error_t err = HURD_DPORT_USE (fd, __file_sync (port, 1, 0));
if (err)
-return __hurd_dfail (fd, err);
+{
+ if (err == EOPNOTSUPP)
+ /* If the file descriptor does not support sync, return EINVAL
+ as POSIX specifies. */
+ err = EINVAL;
+ return __hurd_dfail (fd, err);
+}
return 0;
}
signature.asc
Description: This is a digitally signed message part.
[PATCH] libpthread: fix compatibility as addon with glibc 2.16
Hi, since 2.16, glibc dropped support for any binary format other than ELF; the removal of the 'elf' variable causes libpthread.so to not link, since ld.so is not passed to the linking line. The attached patch defines elf to yes if not defined, allowing to keep compatibility also with glibc < 2.16. Also, I'm slightly in doubt about the ChangeLog snippet, not totally sure about how to write it properly in this case. -- Pino Toscano From 64d00b6482f03f7dfb6f3d4775a9bf44e53cec63 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Fri, 16 Nov 2012 15:04:29 +0100 Subject: [PATCH] Set elf=yes if not defined already glibc 2.16 dropped support for any binary format other than ELF, and the ELF variable has been removed too; this causes a build failure when built as glibc addon with glibc >= 2.16, since it would not link to ld.so. Defining ELF to 'yes' if not defined allows to retain compatibility with older glibc versions. * Makefile [$(..) != ''] [$(elf) = ''] (elf): Define to 'yes'. --- Makefile |4 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index fdfe3ae..c1d8d33 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,10 @@ IN_GLIBC = no else # glibc build IN_GLIBC = yes +# set elf=yes, to retain compatibility with glibc < 2.16 +ifeq ($(elf),) +elf = yes +endif endif ifeq ($(IN_GLIBC),no) -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] ignore Mach kernel headers in check-local-headers.sh
Hi,
the attached patch makes check-local-headers.sh ignore headers from the
"mach" subdirectory, since they the Mach kernel headers used to build
glibc on Hurd.
Thanks,
--
Pino Toscano
check-local-headers: ignore Mach kernel headers
2012-11-16 Pino Toscano
* scripts/check-local-headers.sh: Ignore 'mach' headers.
--- a/scripts/check-local-headers.sh
+++ b/scripts/check-local-headers.sh
@@ -27,12 +27,13 @@ shopt -s nullglob
# Search all dependency files for file names in the include directory.
# There are a few system headers we are known to use.
-# These include Linux kernel headers (asm*, arch, and linux).
+# These include Linux kernel headers (asm*, arch, and linux),
+# and Mach kernel headers (mach).
exec ${AWK} -v includedir="$includedir" '
BEGIN {
status = 0
exclude = "^" includedir \
-"/(.*-.*-.*/|)(asm[-/]|arch|linux/|selinux/|gd|nss3/|c\\+\\+/|sys/(capability|sdt(|-config))\\.h|libaudit\\.h)"
+"/(.*-.*-.*/|)(asm[-/]|arch|linux/|selinux/|mach/|gd|nss3/|c\\+\\+/|sys/(capability|sdt(|-config))\\.h|libaudit\\.h)"
}
/^[^ ]/ && $1 ~ /.*:/ { obj = $1 }
{
signature.asc
Description: This is a digitally signed message part.
[PATCH,HURD] fix muntrace with mmap-less libio
Hi,
I was debugging on Hurd a misbehaviour of muntrace, which would just
spin taking 100% CPU: using a simple test like:
--v--
#include
int main()
{ mtrace(); muntrace(); return 0; }
--^--
when run as `MALLOC_TRACE=out test`, you get a backtrace like the
attached trace.log (that one has been produced with debian's eglibc
2.13, but it does the same with current glibc).
What happens is the following:
a) muntrace gets called
b) in muntrace, "= End\n" is written to the file, and fclose on it is
called
c) in fclose, free is called to free the FILE*, triggering the free hook
(which is still set)
d) in tr_freehook, lock_and_info is called which locks the lock, and
then tr_where is called
e) in tr_where, fprintf in called, which at some point calls
_IO_file_doallocate
and at this point there is the behaviour difference between Linux and
Hurd: in Linux EXEC_PAGESIZE is provided (by sys/param.h, coming from
linux/param.h), thus in libio/libioP.h _G_HAVE_MMAP is kept, so
ALLOC_BUF and FREE_BUF (used in _IO_file_doallocate) use mmap/munmap. On
the Hurd, however, EXEC_PAGESIZE is provided nowhere (and this causes
build issues also in two files under elf/), so the two _BUF macros use
malloc/free... which during the muntrace execution in turn calls the
malloc hook (which is still set), and then lock_and_info tries to lock
the lock -> deadlock.
Ignoring the fact that in libio mmap is not used on Hurd (it will need a
different fix), it seems to me this whole hook triggering during
muntrace seems more harmful than useful (on Linux it is attempted to
output the log for the free of mallstream, which always fails since that
stream is closed at that point), so my attached proposal is to first
unset mallstream and the hooks, and only after that close the file.
Thanks,
--
Pino Toscano
#0 0x0106281c in swtch_pri () at /build/buildd-eglibc_2.13-36-hurd-i386-IvO_gk/eglibc-2.13/build-tree/hurd-i386-libc/mach/swtch_pri.S:2
#1 0x010640a4 in __spin_lock_solid (lock=0x11ded74) at spin-solid.c:27
#2 0x0106429d in __mutex_lock_solid (lock=0x11ded74) at mutex-solid.c:31
#3 0x010eb328 in __mutex_lock (__lock=) at ../mach/lock-intern.h:89
#4 tr_mallochook (size=8192, caller=0x10d690f) at mtrace.c:170
#5 0x010e92ff in __libc_malloc (bytes=8192) at malloc.c:3622
#6 0x010d690f in _IO_file_doallocate (fp=0x804a0e8) at filedoalloc.c:120
#7 0x010e1ec8 in _IO_doallocbuf (fp=0x804a0e8) at genops.c:423
#8 0x010e11db in _IO_new_file_overflow (f=0x804a0e8, ch=-1) at fileops.c:850
#9 0x010e04f3 in _IO_new_file_xsputn (f=0x804a0e8, data=0x11c4665, n=2) at fileops.c:1358
#10 0x010b642e in _IO_vfprintf_internal (s=0x804a0e8, format=0x11c4665 "@ %s%s%s[%p] ", ap=0x1024cf0 "Ч\002\001\344I\034\001") at vfprintf.c:1336
#11 0x010c024b in __fprintf (stream=0x804a0e8, format=0x11c4665 "@ %s%s%s[%p] ") at fprintf.c:33
#12 0x010eb0e0 in tr_where (caller=0x10d6a5c) at mtrace.c:127
#13 0x010eb3a6 in tr_freehook (ptr=0x804a0e8, caller=0x10d6a5c) at mtrace.c:146
#14 0x010e914c in __libc_free (mem=0x1) at malloc.c:3699
#15 0x010d6a5c in _IO_new_fclose (fp=0x804a0e8) at iofclose.c:88
#16 0x010eadcc in muntrace () at mtrace.c:364
#17 0x0804852c in main () at mt.c:3
muntrace: reset file and hooks before finalizing the stream
fclose will call free, invoking its hook, then fprintf which would indirectly
try to allocate a buffer, and this can cause malloc to be used (thus its hook
to be invoked) if libio uses malloc instead of mmap; given any malloc/free hook
locks the internal lock, this leads to a deadlock.
To prevent this hook roundtrip at muntrace, first unset MALLSTREAM and the
hooks, and only after that close the trace file.
2012-11-17 Pino Toscano
* malloc/mtrace.c (muntrace): Reset MALLSTREAM and the hooks before
finalizing MALLSTREAM.
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -361,14 +361,18 @@ mtrace ()
void
muntrace ()
{
+ FILE *f;
+
if (mallstream == NULL)
return;
- fprintf (mallstream, "= End\n");
- fclose (mallstream);
+ f = mallstream;
mallstream = NULL;
__free_hook = tr_old_free_hook;
__malloc_hook = tr_old_malloc_hook;
__realloc_hook = tr_old_realloc_hook;
__memalign_hook = tr_old_memalign_hook;
+
+ fprintf (f, "= End\n");
+ fclose (f);
}
signature.asc
Description: This is a digitally signed message part.
[PATCH] simplify ulimit implementation
Hi,
the current ulimit implementation used for Hurd,
sysdeps/unix/bsd/ulimit.c, is buggy (produces wrong return values,
different function name vs weak alias), while the one for Linux
(sysdeps/unix/sysv/linux/ulimit.c) is better; thus, I decided to remove
the bsd implementation and move the linux one as unix sysdeps (I
initially thought as posix, but since it implements non-standard values
I went for unix).
The only drawback of this operation is that for non-Linux
implementations, using bsd sysdeps and not providing an own ulimit, lose
the (non-standard) __UL_GETMAXBRK case. Can this be considered
acceptable?
Thanks,
--
Pino Toscano
Simplify ulimit implementations
The ulimit implementation in sysdeps/unix/bsd/ulimit.c produces wrong return
values, while sysdeps/unix/sysv/linux/ulimit.c is generally better. Thus,
copy the latter into a more general sysdeps/unix/ulimit.c, and remove the
former.
The only regression for non-Linux implementations using bsd sysdeps and not
providing an own ulimit is that the __UL_GETMAXBRK case (which is non-standard)
is left unimplemented (giving EINVAL).
2012-11-18 Pino Toscano
* sysdeps/unix/sysv/linux/ulimit.c: Moved to ...
* sysdeps/unix/ulimit.c: ... this.
* sysdeps/unix/bsd/ulimit.c: Remove file.
--- /dev/null
+++ b/sysdeps/unix/ulimit.c
@@ -0,0 +1,89 @@
+/* Copyright (C) 1991-2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Function depends on CMD:
+ 1 = Return the limit on the size of a file, in units of 512 bytes.
+ 2 = Set the limit on the size of a file to NEWLIMIT. Only the
+ super-user can increase the limit.
+ 4 = Return the maximum number of files that the calling process
+ can open.
+ Returns -1 on errors. */
+long int
+__ulimit (int cmd, ...)
+{
+ struct rlimit limit;
+ va_list va;
+ long int result = -1;
+
+ va_start (va, cmd);
+
+ switch (cmd)
+{
+case UL_GETFSIZE:
+ /* Get limit on file size. */
+ if (__getrlimit (RLIMIT_FSIZE, &limit) == 0)
+ /* Convert from bytes to 512 byte units. */
+ result = (limit.rlim_cur == RLIM_INFINITY
+ ? LONG_MAX : limit.rlim_cur / 512);
+ break;
+
+case UL_SETFSIZE:
+ /* Set limit on file size. */
+ {
+ long int newlimit = va_arg (va, long int);
+ long int newlen;
+
+ if ((rlim_t) newlimit > RLIM_INFINITY / 512)
+ {
+ limit.rlim_cur = RLIM_INFINITY;
+ limit.rlim_max = RLIM_INFINITY;
+ newlen = LONG_MAX;
+ }
+ else
+ {
+ limit.rlim_cur = newlimit * 512;
+ limit.rlim_max = newlimit * 512;
+ newlen = newlimit;
+ }
+
+ result = __setrlimit (RLIMIT_FSIZE, &limit);
+ if (result != -1)
+ result = newlen;
+ }
+ break;
+
+case __UL_GETOPENMAX:
+ result = __sysconf (_SC_OPEN_MAX);
+ break;
+
+default:
+ __set_errno (EINVAL);
+}
+
+ va_end (va);
+
+ return result;
+}
+
+weak_alias (__ulimit, ulimit);
--- a/sysdeps/unix/bsd/ulimit.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/* Copyright (C) 1991,1992,1994-1998,2001,2005 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-
-extern int _etext;
-
-/* Function depends on CMD:
- 1 = Return the limit on the size of a file, in units of 512 bytes.
- 2 = Set the limit on the size of a file to NEWLIMIT. Only the
- super-user can increase the limit.
- 3 = Return the maximum possible address of the data segment.
- 4 = Return the maximum n
Re: [PATCH,HURD] fix muntrace with mmap-less libio
Hi, (bug-hurd only for this) Alle sabato 17 novembre 2012, Pino Toscano ha scritto: > Ignoring the fact that in libio mmap is not used on Hurd (it will > need a different fix), This has been reported by Thomas Schwinge one year ago: http://thread.gmane.org/87mxd9hl2n@kepler.schwinge.homeip.net (I did not mention that in my previous email, since it was about the muntrace issue.) -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH,HURD] implement syncfs
Hi,
simple implementation of the Linux-ish syncfs on Hurd.
Thanks,
--
Pino Toscano
Hurd: implement syncfs
2012-11-19 Pino Toscano
* sysdeps/mach/hurd/syncfs.c: New file.
--- /dev/null
+++ b/sysdeps/mach/hurd/syncfs.c
@@ -0,0 +1,33 @@
+/* Copyright (C) 2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+
+/* Make all changes done to all files on the file system associated
+ with FD actually appear on disk. */
+int
+syncfs (int fd)
+{
+ /* This is not actually synchronous; we don't wait. */
+ error_t err = HURD_DPORT_USE (fd, __file_syncfs (port, 0, 0));
+ if (err)
+return __hurd_dfail (fd, err);
+ return 0;
+}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH] simplify ulimit implementation
Alle lunedì 19 novembre 2012, Roland McGrath ha scritto: > sysdeps/posix is appropriate because it's implemented in terms of > POSIX interfaces. OK. > It's bad to lose any functionality, though __UL_GETMAXBRK actually > being used seems a bit unlikely. But it's easy enough to add it. Please note that in the linux implementation that case has not bee implemented on purpose: in d2f5be2a1235061b46c51d7530264d086eca46ef that implementation was copied from the bsd one, and the case for 3 (__UL_GETMAXBRK) removed (with the comment adjusted). If __UL_GETMAXBRK is really wanted, I could propose to implement it in the new posix version, but surrounded within a #ifndef ULIMIT_SKIP___UL_GETMAXBRK (or better names welcome), and then replace the whole linux implementation with: #define ULIMIT_SKIP___UL_GETMAXBRK #include (my initial idea was something close to the way e.g. the linux sysconf implementation "overrides" the posix one, but the fact that ulimit takes varargs complicates that approach a bit.) > So I'd do: First commit: move file verbatim to sysdeps/posix/. > Second commit: add __UL_GETMAXBRK to that implementation. > Third commit: remove sysdeps/unix/bsd/ file. OK, I will split the commits once approved. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH,HURD] implement syncfs
Hi,
Alle lunedì 19 novembre 2012, Pino Toscano ha scritto:
> simple implementation of the Linux-ish syncfs on Hurd.
Updated patch according to Roland's and Christoph's comments (thanks!)
--
Pino Toscano
Hurd: implement syncfs
2012-11-19 Pino Toscano
* sysdeps/mach/hurd/syncfs.c: New file.
--- /dev/null
+++ b/sysdeps/mach/hurd/syncfs.c
@@ -0,0 +1,31 @@
+/* Make all changes done to all files on the file system associated
+ with FD actually appear on disk.
+ Copyright (C) 2012 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+
+int
+syncfs (int fd)
+{
+ error_t err = HURD_DPORT_USE (fd, __file_syncfs (port, 1, 0));
+ if (err)
+return __hurd_dfail (fd, err);
+ return 0;
+}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] fix muntrace with mmap-less libio
Alle lunedì 19 novembre 2012, Roland McGrath ha scritto:
> That change is good but it needs a comment. You don't need to go
> into great detail, just say that we call fprintf+fclose after
> clearing the hooks to match how mtrace called fopen+fprintf before
> setting them.
A bit of comment added.
--
Pino Toscano
muntrace: reset file and hooks before finalizing the stream
fclose will call free, invoking its hook, then fprintf which would indirectly
try to allocate a buffer, and this can cause malloc to be used (thus its hook
to be invoked) if libio uses malloc instead of mmap; given any malloc/free hook
locks the internal lock, this leads to a deadlock.
To prevent this hook roundtrip at muntrace, first unset MALLSTREAM and the
hooks, and only after that close the trace file.
2012-11-19 Pino Toscano
* malloc/mtrace.c (muntrace): Reset MALLSTREAM and the hooks before
finalizing MALLSTREAM.
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -361,14 +361,21 @@ mtrace ()
void
muntrace ()
{
+ FILE *f;
+
if (mallstream == NULL)
return;
- fprintf (mallstream, "= End\n");
- fclose (mallstream);
+ /* Do the reverse of what done in mtrace: first reset the hooks and
+ MALLSTREAM, and only after that write the trailer and close the
+ file. */
+ f = mallstream;
mallstream = NULL;
__free_hook = tr_old_free_hook;
__malloc_hook = tr_old_malloc_hook;
__realloc_hook = tr_old_realloc_hook;
__memalign_hook = tr_old_memalign_hook;
+
+ fprintf (f, "= End\n");
+ fclose (f);
}
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurd: compliance fixes for ptsname_r
Hi,
Alle venerdì 20 luglio 2012, Roland McGrath ha scritto:
> > Ok, I see that its `buf' argument is marked nonnull. I added that
> > check because I saw the gnulib test for it explicitly checking
> > that ptsname_r(fd, NULL, 0) would be properly failing with EINVAL
> > (and the man page even explicitly mention that return value,
> > unlike with basically most of the other functions). Should gnulib
> > do that check only on Linux, then?
>
> Well, everybody's wrong. The libc manual never said that you can
> pass NULL and expect not to crash, and the man page was IMHO wrong
> to document it that way. The other implementations never should
> have checked for NULL, but they have done so for a long time.
> gnulib never should have passed NULL to this function and IMHO it
> should be fixed not to do so. But given the history, the least of
> avaialble evils is to make the Hurd implementation consistent with
> the others and do the check.
(few months later... I forgot I sent this patch, so I'm bring it again.)
I updated the patch; is it okay to commit, or should I bring back the
buf==NULL check?
--
Pino Toscano
Hurd: fixes for ptsname and ptsname_r
ptsname_r on failure returns the value that is also set as errno; furthermore,
add more checks to it:
- set errno and return it on __term_get_peername failure
- set errno to ERANGE other than returning it
- change the type of PEERNAME to string_t, and check its length with __strnlen
In ptsname:
- change the type of PEERNAME to string_t
- do not set errno manually, since ptsname_r has set it already
2012-11-19 Pino Toscano
* sysdeps/mach/hurd/ptsname.c (ptsname): Change the type of PEERNAME to
string_t. Do not manually set errno.
(__ptsname_r): Change the type of PEERNAME to string_t, and check its
length with __strnlen. Make sure to both set errno and return it on
failure.
--- a/sysdeps/mach/hurd/ptsname.c
+++ b/sysdeps/mach/hurd/ptsname.c
@@ -29,12 +29,10 @@
char *
ptsname (int fd)
{
- static char peername[1024]; /* XXX */
+ static string_t peername;
error_t err;
err = __ptsname_r (fd, peername, sizeof (peername));
- if (err)
-__set_errno (err);
return err ? NULL : peername;
}
@@ -46,17 +44,19 @@ ptsname (int fd)
int
__ptsname_r (int fd, char *buf, size_t buflen)
{
- char peername[1024]; /* XXX */
+ string_t peername;
size_t len;
error_t err;
- peername[0] = '\0';
if (err = HURD_DPORT_USE (fd, __term_get_peername (port, peername)))
-return _hurd_fd_error (fd, err);
+return __hurd_dfail (fd, err), errno;
- len = strlen (peername) + 1;
+ len = __strnlen (peername, sizeof peername - 1) + 1;
if (len > buflen)
-return ERANGE;
+{
+ errno = ERANGE;
+ return ERANGE;
+}
memcpy (buf, peername, len);
return 0;
signature.asc
Description: This is a digitally signed message part.
Re: hurd-20120710 FTBFS due to missing dependencies
Hi, (note that this is debian-hurd@ material, since it is specific to the Debian packaging.) Alle lunedì 26 novembre 2012, Svante Signell ha scritto: > When trying to build hurd-20120710 from source on a new box the build > failed due to missing dependencies on flex|bison, pkg-config and > libx11-dev (neither was installed by apt-get build-dep hurd, or > build-essential or devscripts). Those have been added by Samuel in the packaging repository some time ago, and will be part of the next upload. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
[PATCH] procfs: simple implementation of statfs
Hi,
attached there is a small patch for procfs to implement statfs,
returning only the two fields we can fill for sure.
May I push it?
Thanks,
--
Pino Toscano
From 1b7ad5c5d601b6388f0fc871b4fa42d231c05400 Mon Sep 17 00:00:00 2001
From: Pino Toscano
Date: Thu, 6 Dec 2012 17:51:58 +0100
Subject: [PATCH] Simple implementation of statfs
Initial implementation of statfs reply, just returning the filesystem type and its id.
* netfs.c: Include and .
(netfs_attempt_statfs): Implement.
---
netfs.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/netfs.c b/netfs.c
index a4d9f9c..c139d11 100644
--- a/netfs.c
+++ b/netfs.c
@@ -23,6 +23,8 @@
#include
#include
#include
+#include
+#include
#include "procfs.h"
#define PROCFS_SERVER_NAME "procfs"
@@ -347,7 +349,10 @@ error_t netfs_attempt_set_size (struct iouser *cred, struct node *np,
error_t netfs_attempt_statfs (struct iouser *cred, struct node *np,
fsys_statfsbuf_t *st)
{
- return ENOSYS;
+ memset (st, 0, sizeof *st);
+ st->f_type = FSTYPE_PROC;
+ st->f_fsid = getpid ();
+ return 0;
}
/* The user must define this function. This should sync the locked
--
1.7.10.4
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH] procfs: simple implementation of statfs
Alle giovedì 6 dicembre 2012, Thomas Schwinge ha scritto: > Hi! > > On Thu, 6 Dec 2012 17:57:32 +0100, Pino Toscano wrote: > > Initial implementation of statfs reply, just returning the > > filesystem type and its id. > > > > * netfs.c: Include and . > > (netfs_attempt_statfs): Implement. > > OK; same is done in ftpfs. Thanks, pushed. > What does this not-totally-anymore-but-still-quite stub improve -- > how did you observe this issue/what is it useful for? Nothing much, I was playing with `stat`. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH] simplify ulimit implementation
Hi,
Alle lunedì 19 novembre 2012, Roland McGrath ha scritto:
> I don't think losing __UL_GETMAXBRK is really a problem. The reason
> for removing it was not specific to Linux, just to the
> implementation being in a shared library. It could be implemented
> in a shared library, at the cost of a GOT reloc for _etext to get
> the main executable's value (or conversely, perhaps some grovelling
> in dynamic linker data structures for the shared case). But given
> that the current implementation usable on the Hurd will yield
> utterly useless values, the case for having it at all is
> inordinately weak.
OK.
Attached there are the two patches of the ulimit reorganization, as you
suggested in a previous email:
1) move the linux implementation as posix (including limits.h)
2) remove the bsd implementation
Thanks,
--
Pino Toscano
ulimit: move linux implementation as posix
The linux implementation of ulimit works correctly and has nothing specific
to Linux, so move it as general posix implementation.
2013-01-15 Pino Toscano
* sysdeps/unix/sysv/linux/ulimit.c: Moved to ...
* sysdeps/posix/ulimit.c: ... this.
Include .
--- /dev/null
+++ b/sysdeps/posix/ulimit.c
@@ -0,0 +1,90 @@
+/* Copyright (C) 1991-2013 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Function depends on CMD:
+ 1 = Return the limit on the size of a file, in units of 512 bytes.
+ 2 = Set the limit on the size of a file to NEWLIMIT. Only the
+ super-user can increase the limit.
+ 4 = Return the maximum number of files that the calling process
+ can open.
+ Returns -1 on errors. */
+long int
+__ulimit (int cmd, ...)
+{
+ struct rlimit limit;
+ va_list va;
+ long int result = -1;
+
+ va_start (va, cmd);
+
+ switch (cmd)
+{
+case UL_GETFSIZE:
+ /* Get limit on file size. */
+ if (__getrlimit (RLIMIT_FSIZE, &limit) == 0)
+ /* Convert from bytes to 512 byte units. */
+ result = (limit.rlim_cur == RLIM_INFINITY
+ ? LONG_MAX : limit.rlim_cur / 512);
+ break;
+
+case UL_SETFSIZE:
+ /* Set limit on file size. */
+ {
+ long int newlimit = va_arg (va, long int);
+ long int newlen;
+
+ if ((rlim_t) newlimit > RLIM_INFINITY / 512)
+ {
+ limit.rlim_cur = RLIM_INFINITY;
+ limit.rlim_max = RLIM_INFINITY;
+ newlen = LONG_MAX;
+ }
+ else
+ {
+ limit.rlim_cur = newlimit * 512;
+ limit.rlim_max = newlimit * 512;
+ newlen = newlimit;
+ }
+
+ result = __setrlimit (RLIMIT_FSIZE, &limit);
+ if (result != -1)
+ result = newlen;
+ }
+ break;
+
+case __UL_GETOPENMAX:
+ result = __sysconf (_SC_OPEN_MAX);
+ break;
+
+default:
+ __set_errno (EINVAL);
+}
+
+ va_end (va);
+
+ return result;
+}
+
+weak_alias (__ulimit, ulimit);
--- a/sysdeps/unix/sysv/linux/ulimit.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/* Copyright (C) 1991-2013 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-/* Function depends on CMD:
- 1 = Return the limit on the size of a file, in units of 512 bytes.
- 2 = Set the limit on the size of a file to NEWLIMIT. Only the
- super-user can increase the limit.
- 3 = illegal due to shared libraries; normally is
- (Return the maximum possible address of the data segment.)
- 4 = Return the maximum number of files that the calling process
- can open
Re: [PATCH,HURD] hurdselect: Step1, code split preparations
Hi,
Alle martedì 22 gennaio 2013, Svante Signell ha scritto:
> Attached is the first patch for a 3-way split of hurdselect.c into
> three cases: DELAY, POLL, SELECT
What's the use of of the separate DELAY case?
Basically, it seems to be optimizing just a very specific case, i.e.:
select(0, ..., &timeout);
while, on the other hand, code like
fd_set empty;
FD_ZERO(&empty);
select(1, &empty, &empty, &empty, &timeout);
(or with any nfd > 0) with your changes would be considered as SELECT.
After all, the code checking the arguments should take care of handling
the "no descriptors" situations already, so the separate DELAY is
simply redundant (being a subset of SELECT).
> leading to a more POSIX conforming POLL.
theorically, poll could be fixed also without restructing hurdselect...
> Starting point is the hurdselect.c created by all Debian patches
> applied up to eglibc-2.13-38 + 3 additional patches:
Note that if you want to send patches upstream, your code must apply
on master branch of glibc.git, not on some old glibc version with
distribution-specific patches.
> + union
> + {
> [...]
This block must be properly indented.
> const int fd = (int) d[i].io_port;
>
> - if (fd < _hurd_dtablesize)
> - {
> d[i].cell = _hurd_dtable[fd];
> d[i].io_port = _hurd_port_get (&d[i].cell->port,
> &d[i].ulink); if (d[i].io_port != MACH_PORT_NULL)
> continue;
> - }
why is this check removed? When collecting inputs from poll,
d[i].io_port is the fd passed, which has no guarantee to be lower than
_hurd_dtablesize.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH,HURD] hurdselect: Step1, code split preparations
Alle martedì 22 gennaio 2013, Svante Signell ha scritto: > On Tue, 2013-01-22 at 19:15 +0100, Pino Toscano wrote: > > Alle martedì 22 gennaio 2013, Svante Signell ha scritto: > > > Attached is the first patch for a 3-way split of hurdselect.c > > > into three cases: DELAY, POLL, SELECT > > > > What's the use of of the separate DELAY case? > > > > Basically, it seems to be optimizing just a very specific case, i.e.: > > select(0, ..., &timeout); > > > > while, on the other hand, code like > > > > fd_set empty; > > FD_ZERO(&empty); > > select(1, &empty, &empty, &empty, &timeout); > > > > (or with any nfd > 0) with your changes would be considered as > > SELECT. > > If nfds>0 it depends on pollfds if the case is POLL or SELECT? > > From the man page for select: > Some code calls select() with all three sets empty, nfds zero, and a > non-NULL timeout as a fairly portable way to sleep with subsecond > precision. I know what the select documentation says; what I said also is basically what you didn't reply to: > > After all, the code checking the arguments should take care of > > handling the "no descriptors" situations already, so the separate > > DELAY is simply redundant (being a subset of SELECT). Having a separate case for what is done as part of another one seems redudant to me. > And of course calling poll with nfds=0 is equal to a delay too if > timeout != 0 Actually, from my reading the poll documentation (unlike the select one) does not say what to do when nfds=0; on Linux, the timeout seems ignored and 0 is returned right away, with no delay. -- Pino Toscano signature.asc Description: This is a digitally signed message part.
Re: [PATCH] simplify ulimit implementation
Alle giovedì 17 gennaio 2013, Roland McGrath ha scritto: > First do a commit that is nothing but the rename. That won't affect > any configuration. Then do a commit adding the missing #include > (and test it by locally removing the bsd file and doing a Hurd > build). Finally, do a commit removing the bsd file. Done, I think I should have done everything correctly. The addition of was needed on Hurd (I tested it before sending the first version of this ulimit rework) -- in any case, LONG_MAX is explicitly used, so that include is not wrong. Thanks, -- Pino Toscano signature.asc Description: This is a digitally signed message part.
