Re: syncing libc and libkern

2014-06-05 Thread Theo de Raadt
> However, seeing as things are maintained separately (for good
> reasons), perhaps copy-to-libkern itself should just be removed
> since it's basically pointless at this point and hasn't been
> used for about a decade.

I think that is the right direction.  Then, do a seperate cleanup.



Re: syncing libc and libkern

2014-06-05 Thread Jean-Philippe Ouellet
On Wed, Jun 04, 2014 at 08:02:06PM +, Miod Vallat wrote:
> > First, str{cat,cpy} were vehemently expunged from the kernel many years ago,
> > so stop trying to keep them around.
> > 
> > Index: lib/libc/Makefile.inc
> 
> Hello, this is libc you are butchering in. I'm afraid strcat and strcpy
> are still required to be in libc by the current C and POSIX standards.

Yes, that's correct. This doesn't remove str{cat,cpy} from libc
(haha, I wish that were possible), only removes it from the list
of things that are to be copied from libc to libkern.

However, seeing as things are maintained separately (for good
reasons), perhaps copy-to-libkern itself should just be removed
since it's basically pointless at this point and hasn't been
used for about a decade.



Re: syncing libc and libkern

2014-06-04 Thread Miod Vallat
> It seems the correct thing to do here is to put the shared part in libc,
> list the file in KSRCS in lib/libc/Makefile.inc, and make copy-to-libkern.
> This allows for only one copy to need to be maintained, and avoids hard
> build-time dependency tentacles between lib/ and sys/.
> (If this is not correct, let me know what's better.)
> 
> Unfortunately, libc and libkern haven't been kept in sync for many years
> and I don't see a technical reason why not, so lets bring them together?

That's a slippery slope you're walking on.

>From a theoretical point of view, the routines common to libc and
libkern should be identical.

>From a practical point of view, this is not always the case. I'll only
cite the two most important reasons:
1. libc is a library, and is required to provide entry points for all
   its interfaces (such as htonl), so that their address may be used for
   function pointers, even if the function itself is a no-op or would
   better be an inline; libkern has no such constraints.
2. libc is intended to run in userland, and libkern, well, in the
   kernel; running in the kernel might require different code constructs
   and/or will allow for privileged instructions to be used (especially
   if address spaces are kept completely separate between kernel space
   and user space)

> First, str{cat,cpy} were vehemently expunged from the kernel many years ago,
> so stop trying to keep them around.
> 
> Index: lib/libc/Makefile.inc

Hello, this is libc you are butchering in. I'm afraid strcat and strcpy
are still required to be in libc by the current C and POSIX standards.

> Next, __P is dead. Theo just removed the mention of it from style(9), and
> this happens to be the last one in libc. It was removed from the libkern
> copy 9 years ago, but that change never made it "upstream" to libc.
> 
> Index: lib/libc/quad/qdivrem.c

This one I agree with.

> bcmp(3) was cleaned up in libc, but that never propogated to libkern.
> While here, remove a comment that doesn't belong in the MI version.
> 
> This does change the functionality a bit, but anything relying on undefined
> behavior or specific value of "non-zero" here is fundamentally broken, and
> I didn't see anything making such assumptions on a quick grep through sys/.
> 
> Index: sys/lib/libkern/bcmp.c

This one brings bad memories, as memcmp() in the kernel used to map to
bcmp() on vax, and this broke red-black trees. But having libkern here
match libc (at least on the platforms where the kernel bcmp() is
provided by this file) will not hurt.

> {h,n}to{n,h}{l,s} and bswap32 have been broken up into different files in 
> libc,
> but I'm not sure how to properly break that up into MI/MD components for 
> libkern
> and include it in the right places without breaking anything, does somebody 
> else
> want to take a look?

See comment about different rules between libc and libkern. Kernels for
platforms with the right byte order need not actually contain these
symbols.

> Also, for future reference, would these patches be preferred in separate 
> emails?

Preferrably (and lines trimmed to the usual 72 columns). Although for
simple diffs, this may be acceptable.



syncing libc and libkern

2014-06-03 Thread Jean-Philippe Ouellet
Hello,

This came up when I was looking for the proper place to put code for dealing
with capsicum data structures which need to be handled by both userland and
the kernel.

FreeBSD's libc build system has tentacles that reach over and grab
sys/kern/subr_capability.c. That's not very elegant, I like how
our tree is nicely separated.

It seems the correct thing to do here is to put the shared part in libc,
list the file in KSRCS in lib/libc/Makefile.inc, and make copy-to-libkern.
This allows for only one copy to need to be maintained, and avoids hard
build-time dependency tentacles between lib/ and sys/.
(If this is not correct, let me know what's better.)

Unfortunately, libc and libkern haven't been kept in sync for many years
and I don't see a technical reason why not, so lets bring them together?


First, str{cat,cpy} were vehemently expunged from the kernel many years ago,
so stop trying to keep them around.

Index: lib/libc/Makefile.inc
===
RCS file: /cvs/src/lib/libc/Makefile.inc,v
retrieving revision 1.19
diff -u -p -r1.19 Makefile.inc
--- lib/libc/Makefile.inc   12 May 2014 19:25:16 -  1.19
+++ lib/libc/Makefile.inc   3 Jun 2014 08:09:25 -
@@ -61,8 +61,8 @@ CFLAGS+=-DNLS
 
 LIBKERN=   ${LIBCSRCDIR}/../../sys/lib/libkern
 
-KSRCS= bcmp.c bzero.c ffs.c strcat.c strcmp.c strcpy.c strlen.c  strncmp.c \
-   strncpy.c strnlen.c htonl.c htons.c ntohl.c ntohs.c timingsafe_bcmp.c
+KSRCS= bcmp.c bzero.c ffs.c strcmp.c strlen.c  strncmp.c strncpy.c \
+   strnlen.c htonl.c htons.c ntohl.c ntohs.c timingsafe_bcmp.c
 .if (${MACHINE_CPU} != "alpha")
 KSRCS+=adddi3.c anddi3.c ashldi3.c ashrdi3.c cmpdi2.c divdi3.c 
iordi3.c \
lshldi3.c lshrdi3.c moddi3.c muldi3.c negdi2.c notdi2.c qdivrem.c \


Next, __P is dead. Theo just removed the mention of it from style(9), and
this happens to be the last one in libc. It was removed from the libkern
copy 9 years ago, but that change never made it "upstream" to libc.

Index: lib/libc/quad/qdivrem.c
===
RCS file: /cvs/src/lib/libc/quad/qdivrem.c,v
retrieving revision 1.7
diff -u -p -r1.7 qdivrem.c
--- lib/libc/quad/qdivrem.c 8 Aug 2005 08:05:35 -   1.7
+++ lib/libc/quad/qdivrem.c 3 Jun 2014 08:09:19 -
@@ -51,7 +51,7 @@ typedef unsigned short digit;
 typedef u_int digit;
 #endif
 
-static void shl __P((digit *p, int len, int sh));
+static void shl(digit *p, int len, int sh);
 
 /*
  * __qdivrem(u, v, rem) returns u/v and, optionally, sets *rem to u%v.


bcmp(3) was cleaned up in libc, but that never propogated to libkern.
While here, remove a comment that doesn't belong in the MI version.

This does change the functionality a bit, but anything relying on undefined
behavior or specific value of "non-zero" here is fundamentally broken, and
I didn't see anything making such assumptions on a quick grep through sys/.

Index: sys/lib/libkern/bcmp.c
===
RCS file: /cvs/src/sys/lib/libkern/bcmp.c,v
retrieving revision 1.9
diff -u -p -r1.9 bcmp.c
--- sys/lib/libkern/bcmp.c  27 Oct 2009 23:59:35 -  1.9
+++ sys/lib/libkern/bcmp.c  3 Jun 2014 08:11:42 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: bcmp.c,v 1.9 2009/10/27 23:59:35 deraadt Exp $*/
+/* $OpenBSD: bcmp.c,v 1.9 2008/03/19 03:00:23 ray Exp $*/
 
 /*
  * Copyright (c) 1987 Regents of the University of California.
@@ -35,23 +35,18 @@
 #include 
 #endif
 
-/*
- * bcmp -- vax cmpc3 instruction
- */
 int
-bcmp(b1, b2, length)
-   const void *b1, *b2;
-   register size_t length;
+bcmp(const void *b1, const void *b2, size_t length)
 {
-   register char *p1, *p2;
+   char *p1, *p2;
 
if (length == 0)
-   return(0);
+   return (0);
p1 = (char *)b1;
p2 = (char *)b2;
do
if (*p1++ != *p2++)
-   break;
+   return (1);
while (--length);
-   return(length);
+   return (0);
 }
Index: lib/libc/string/bcmp.c
===
RCS file: /cvs/src/lib/libc/string/bcmp.c,v
retrieving revision 1.9
diff -u -p -r1.9 bcmp.c
--- lib/libc/string/bcmp.c  19 Mar 2008 03:00:23 -  1.9
+++ lib/libc/string/bcmp.c  3 Jun 2014 08:11:42 -
@@ -35,9 +35,6 @@
 #include 
 #endif
 
-/*
- * bcmp -- vax cmpc3 instruction
- */
 int
 bcmp(const void *b1, const void *b2, size_t length)
 {


{h,n}to{n,h}{l,s} and bswap32 have been broken up into different files in libc,
but I'm not sure how to properly break that up into MI/MD components for libkern
and include it in the right places without breaking anything, does somebody else
want to take a look?

Also, for future reference, would these patches be preferred in separate emails?