Bug 80587

Comment 1 Giorgos Keramidas 2005-05-04 00:00:22 UTC

On 2005-05-03 20:35, Arne H Juul <arnej@europe.yahoo-inc.com> wrote:
> We had a problem where a daemon wouldn't read commands on its socket;
> after some debugging we found that accept() on the socket didn't work
> (with errno EINVAL).  According to man 2 accept this can only be
> caused by "listen(2) has not been called" but after more intensive
> debugging we couldn't figure out how that was possible in our program.
>
> Inspection of the actual kernel source for accept(2) however shows
> that it also returns EINVAL when the namelen argument is negative; it
> turns out the program had a stupid bug where the namelen was
> uninitialized causing accept() to fail unpredicably depending on stack
> contents.  The following documentation fix would (hopefully) have
> saved us some time chasing down codepaths through listen().

True!  The relevant bits in CURRENT are in uipc_syscalls.c (which you
already know, but I'm posting here for the audit trail):

    281 static int
    282 accept1(td, uap, compat)
    ...
    290 {
    ...
    307                 if (namelen < 0)
    308                         return (EINVAL);

>  .It Bq Er EINVAL
>  .Xr listen 2
>  has not been called on the socket descriptor.
> +.It Bq Er EINVAL
> +The
> +.Fa addrlen
> +argument is negative.

I think that documenting both cases in a single paragraph of accept(2)
is ok too:

	 .It Bq Er EINVAL
	 .Xr listen 2
	-has not been called on the socket descriptor.
	+has not been called on the socket descriptor or the
	+.Fa addrlen
	+argument is negative.

If this change to the diff is ok with you too, I'll see that it gets
committed.

- Giorgos

Comment 2 Mark Linimon 2005-05-04 02:24:39 UTC

Responsible Changed
From-To: freebsd-bugs->freebsd-doc

Patch provided is against the documentation, not the kernel.

On Wed, 4 May 2005, Giorgos Keramidas wrote:
> I think that documenting both cases in a single paragraph of accept(2)
> is ok too:
>
> 	 .It Bq Er EINVAL
> 	 .Xr listen 2
> 	-has not been called on the socket descriptor.
> 	+has not been called on the socket descriptor or the
> 	+.Fa addrlen
> 	+argument is negative.

This is just a style thing, and personally I don't really mind
either way; other system call man pages have separate paragraphs
for same errno with different reasons, so I think it's best
to follow that style.

-- 
Arne H Juul                       Mail:  arnej@europe.yahoo-inc.com
Release engineer                  Web:   http://www.yahoo.com/
Yahoo Norway                      Phone: +47 7320 1219
Prinsensgate 49, 7013 Trondheim   Fax:   +47 7320 1201

Comment 4 Giorgos Keramidas 2005-05-04 12:09:32 UTC

State Changed
From-To: open->closed

The original diff has been committed to CURRENT.  In a few days I'll merge 
this to -STABLE versions. 

Thanks :) 

Comment 5 Giorgos Keramidas 2005-05-04 12:09:32 UTC

Responsible Changed
From-To: freebsd-doc->keramida

Comment 6 Giorgos Keramidas 2005-05-04 12:10:49 UTC

State Changed
From-To: closed->patched

Keep this open (as 'patched') until I merge it to STABLE.

Comment 7 Remko Lodder 2005-08-20 09:38:09 UTC

State Changed
From-To: patched->closed

I merged it to RELENG_5 per request of rionda. Also make sure 
that I get a pointyhat if something goes wrong instead of 
keramida. 

Comment 8 Remko Lodder 2005-08-20 09:38:09 UTC

Responsible Changed
From-To: keramida->remko

I merged it to RELENG_5 per request of rionda. Also make sure 
that I get a pointyhat if something goes wrong instead of 
keramida.