[PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Roland McGrath]

The OPEN_MAX constant is an arbitrary number with no useful relation to
anything.  Nothing should be using it.  This patch changes SCM_MAX_FD to
use NR_OPEN instead of OPEN_MAX.  This increases the size of the struct
scm_fp_list type fourfold, to make it big enough to contain as many file
descriptors as could be asked of it.  This size increase may not be very
worthwhile, but at any rate if an arbitrary limit unrelated to anything
else is being defined it should be done explicitly here with:

	#define SCM_MAX_FD	255

Using the OPEN_MAX constant here is just confusing and misleading.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 include/net/scm.h |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/scm.h b/include/net/scm.h
index 5637d5e..4d37c5e 100644  
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -8,7 +8,7 @@
 /* Well, we should have at least one descriptor open
  * to accept passed FDs 8)
  */
-#define SCM_MAX_FD	(OPEN_MAX-1)
+#define SCM_MAX_FD	(NR_OPEN-1)
 
 struct scm_fp_list
 {

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Benjamin LaHaise]

On Tue, Mar 13, 2007 at 01:39:12AM -0700, Roland McGrath wrote:
> The OPEN_MAX constant is an arbitrary number with no useful relation to
> anything.  Nothing should be using it.  This patch changes SCM_MAX_FD to
> use NR_OPEN instead of OPEN_MAX.  This increases the size of the struct
> scm_fp_list type fourfold, to make it big enough to contain as many file
> descriptors as could be asked of it.  This size increase may not be very
> worthwhile, but at any rate if an arbitrary limit unrelated to anything
> else is being defined it should be done explicitly here with:

> -#define SCM_MAX_FD	(OPEN_MAX-1)
> +#define SCM_MAX_FD	(NR_OPEN-1)

This is a bad idea.  From linux/fs.h:

#undef NR_OPEN
#define NR_OPEN (1024*1024)     /* Absolute upper limit on fd num */

There isn't anything I can see guaranteeing that net/scm.h is included 
before fs.h.  This affects networking and should really be Cc'd to 
netdev@vger.kernel.org, which will raise the issue that if SCM_MAX_FD is 
raised, the resulting simple kmalloc() must be changed.  That said, I 
doubt SCM_MAX_FD really needs to be raised, as applications using many 
file descriptors are unlikely to try to send their entire file table to 
another process in one go -- they have to handle the limits imposed by 
SCM_MAX_FD anyways.

		-ben
-- 
"Time is of no importance, Mr. President, only life is important."
Don't Email: <zyntrop@kvack.org>.

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Roland McGrath]

> > -#define SCM_MAX_FD	(OPEN_MAX-1)
> > +#define SCM_MAX_FD	(NR_OPEN-1)
> 
> This is a bad idea.  [...]

Ok.  My only agenda is to get rid of OPEN_MAX.
I then propose the following instead.


Thanks,
Roland

---
[PATCH] avoid OPEN_MAX in SCM_MAX_FD

The OPEN_MAX constant is an arbitrary number with no useful relation to
anything.  Nothing should be using it.  SCM_MAX_FD is just an arbitrary
constant and it should be clear that its value is chosen in net/scm.h
and not actually derived from anything else meaningful in the system.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 include/net/scm.h |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/scm.h b/include/net/scm.h
index 5637d5e..2240690 100644  
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -8,7 +8,7 @@
 /* Well, we should have at least one descriptor open
  * to accept passed FDs 8)
  */
-#define SCM_MAX_FD	(OPEN_MAX-1)
+#define SCM_MAX_FD	255
 
 struct scm_fp_list
 {

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Linus Torvalds]

On Tue, 13 Mar 2007, Roland McGrath wrote:
> 
> The OPEN_MAX constant is an arbitrary number with no useful relation to
> anything.  Nothing should be using it.  SCM_MAX_FD is just an arbitrary
> constant and it should be clear that its value is chosen in net/scm.h
> and not actually derived from anything else meaningful in the system.

I'd actually prefer this as part of the "remove OPEN_MAX" patch.

It's certainly nice to have small independent patches in a series, but two 
one-liners that really aren't all that independent either in practice or 
in goals doesn't make much sense to me. Much better to just be up-front 
about things and say: "remove OPEN_MAX, and to do so, just rewrite that 
other arbitrary constant to not need it any more".

That said, it actually worries me that you should call "_SC_OPEN_MAX". I 
think the whole POSIX config method is way over-designed (anybody who 
thinks you should ever have used _SC_HZ or whatever it was called was just 
crazy), but more importantly, and independently of that worry, I just 
suspect a lot of programs simply _don't_do_it_.

For example, I know perfectly well that I should use _SC_PATH_MAX, but a 
*lot* of code simply doesn't care. In git, I used PATH_MAX, and the reason 
is that
 - I want a constant for arrays
 - I don't care that much about the exact value, I just want a reasonable 
   value for sizing an array for some random path
 - _SC_PATH_MAX is practically unportable and simply not *useful*.

.. in short, I'm not a big believer in "programs should do Xyz according 
to some paper standard". Paper standards are written by committees, not 
programmers, and seldom take issues other than politics into account.

So, what's the likelihood that this will break some old programs? I 
realize that modern distributions don't put the kernel headers in their 
user-visible includes any more, but the breakage is most likely exactly 
for old programs and older distributions.

		Linus

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Roland McGrath]

> I'd actually prefer this as part of the "remove OPEN_MAX" patch.

Ok.  (But now you're going to argue with me about "remove OPEN_MAX",
and you haven't said you have any problem with changing SCM_MAX_FD,
so why make it wait?)

> That said, it actually worries me that you should call "_SC_OPEN_MAX". 
[...]
> For example, I know perfectly well that I should use _SC_PATH_MAX, but a 
> *lot* of code simply doesn't care. In git, I used PATH_MAX, and the reason 
[...]

Ok, fine.  But PATH_MAX is a real constant that has some meaning in the
kernel.  It's perfectly correct to use PATH_MAX as a constant on a system
like Linux that defines it and means what it says.  Conversely, OPEN_MAX
has no useful relationship with anything the kernel is doing at all.

> So, what's the likelihood that this will break some old programs? I 
> realize that modern distributions don't put the kernel headers in their 
> user-visible includes any more, but the breakage is most likely exactly 
> for old programs and older distributions.

Well, I don't know for sure.  It doesn't seem all that likely to me (not
like PATH_MAX), as there has been getdtablesize() since before there was
OPEN_MAX by that name (not to mention before there was Linux).  If things
use OPEN_MAX as a constant for arrays, they're already broken unless they
call setrlimit to constrain themselves.  Getting things fixed has to start
somewhere.


Thanks,
Roland

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Linus Torvalds]

On Tue, 13 Mar 2007, Roland McGrath wrote:
> 
> Ok, fine.  But PATH_MAX is a real constant that has some meaning in the
> kernel.  It's perfectly correct to use PATH_MAX as a constant on a system
> like Linux that defines it and means what it says.  Conversely, OPEN_MAX
> has no useful relationship with anything the kernel is doing at all.

Sure. I'm just saying that some people may use OPEN_MAX the way I know 
people use PATH_MAX - whether it's what you're supposed to or not.

I do agree that PATH_MAX is much more appropriate to be used that way, and 
is more likely to have "real" meaning, I just worry.

		Linus

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Jochen Voß]

Hi,

On 14 Mar 2007, at 01:15, Linus Torvalds wrote:
> On Tue, 13 Mar 2007, Roland McGrath wrote:
>> Ok, fine.  But PATH_MAX is a real constant that has some meaning  
>> in the
>> kernel.  It's perfectly correct to use PATH_MAX as a constant on a  
>> system
>> like Linux that defines it and means what it says.  Conversely,  
>> OPEN_MAX
>> has no useful relationship with anything the kernel is doing at all.
>
> Sure. I'm just saying that some people may use OPEN_MAX the way I know
> people use PATH_MAX - whether it's what you're supposed to or not.

To contribute a data point here: searching for the regular expression

     \[\ *OPEN_MAX\ *\]

on google code search [1] gives 25 hits for me.  Replacing OPEN_MAX  
with PATH_MAX results in "about 19,700" hits.

[1] http://www.google.com/codesearch?hl=en&q=+%5C%5B%5C+*OPEN_MAX%5C+* 
%5C%5D&start=20&sa=N

I hope this helps,
Jochen
--
http://seehuhn.de/

Re: [PATCH 1/2] avoid OPEN_MAX in SCM_MAX_FD

[Olaf Kirch]

On Wednesday 14 March 2007 02:15, Linus Torvalds wrote:
> Sure. I'm just saying that some people may use OPEN_MAX the way I know
> people use PATH_MAX - whether it's what you're supposed to or not.

glibc removed OPEN_MAX from its header files several years ago. If you
want to find a piece of code that still uses it, it's most likely something
that hasn't seen a compiler in years - and will likely continue to do
so.

Olaf
-- 
Olaf Kirch  |  --- o --- Nous sommes du soleil we love when we play
okir@lst.de |    / | \   sol.dhoop.naytheet.ah kin.ir.samse.qurax