Re: [PATCH] simplify ulimit implementation

[Pino Toscano]

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.

Re: [PATCH] simplify ulimit implementation

[Roland McGrath]

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.


Thanks,
Roland

Re: [PATCH] simplify ulimit implementation

[Pino Toscano]

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
+   .  */
+
+#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
-   .  */
-
-#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.
-   Returns -1 on errors.  */
-long int
-__ulimit (int cmd, ...)
-

Re: [PATCH] simplify ulimit implementation

[Roland McGrath]

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.

Re: [PATCH] simplify ulimit implementation

[Pino Toscano]

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] simplify ulimit implementation

[Roland McGrath]

sysdeps/posix is appropriate because it's implemented in terms of POSIX
interfaces.

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.  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.


Thanks,
Roland

[PATCH] simplify ulimit implementation

[Pino Toscano]

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
+   .  */
+
+#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
-   .  */
-
-#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 number of files that the ca