[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: kernel on buildd (Re: [Glibc-bsd-commits] r5714 - trunk/glibc-ports/kfreebsd/bits)



Hey Steven!

On Thu, 2015-05-28 at 22:22:06 +0100, Steven Chamberlain wrote:
> Guillem Jover wrote:
> > In any case here's the patches I've prepared to send upstream. Only
> > build tested, though.
> 
> Thanks for this.

Thank you for the review and testing.

> Patch 1 would have been a good workaround for Debian sid, if Andreas
> had accepted it, but I don't think it should be permanent or go
> upstream.  I don't like to special-case kfreebsd in code except in
> places we really have to, and it's actually wrong for FreeBSD <= 9
> (though they're not a user of util-linux).

This allows using the value even if the build system does not know
about it, either because we are building on an old system or because
the libc is not up-to-date. For FreeBSD <= 9, it should be handled
by the run-time checks, so it should fallback to the other codepath.
I can reorder the patches so that this makse more sense.

In any case I think I'll just send it upstream and let them decide if
they want to carry it or not. Or did you mean something else with the
comment that it is wrong (I would not expect the value to have been
reused for example)?

> Patches 2 and 3 look good to go upstream as they benefit other systems
> not having fcntl.h or F_DUPFD_CLOEXEC (and older kfreebsd).

Sure.

> They make patch 1 unnecessary?

Well, partially I guess, if the library is being used by threaded
applications that fork, then they might end up leaking if the library
got built on an old system, but is run on a newer one.

> I tested your fallback code and it is working (duplicates the fd and
> sets FD_CLOEXEC).  Without F_DUPFD_CLOEXEC I get a compiler warning
> about unused lowfd, but that doesn't really matter here.

Cool, thanks. Sending them upstream now.

Regards,
Guillem


Reply to: