[BusyBox] [patch] Cleanup for libbb/loop.c
Rob Landley
rob at landley.net
Wed Nov 17 09:02:18 UTC 2004
On Tuesday 16 November 2004 14:18, Erik Andersen wrote:
> On Tue Nov 16, 2004 at 06:51:39AM -0600, Rob Landley wrote:
> > P.S. Why are we including kernel_types.h in a userspace program
> > at all? Anybody know?
>
> Because we need to define structure which communicates
> between the kernel and user space.
Except that kernel_types is the one that you're supposed to include in kernel
code, and posix_types is the one you're supposed to include in userspace
code.
> > --- busybox/busybox/libbb/loop.c 2004-08-16 04:36:28.000000000 -0400
> > +++ lfs/newbuild/lfs/tools/sources/busybox-new/libbb/loop.c 2004-11-16
> > 05:50:57.000000000 -0500 @@ -19,6 +19,9 @@
> > * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> > USA */
> >
> > +#include <asm/posix_types.h>
> > +#include <linux/loop.h>
>
> No no no no no no no no no!
>
> Rule #1: Never ever include kernel header files in user space.
> Rule #2: See Rule #1
> Rule #3: See Rule #2
But you're violating your own rule by including linux/version.h and having
#ifdefs based off of that, aren't you? You're honestly advocating block
copying random crap out of one of those forbidden header files and then
having a forest of #ifdefs with incestuous knowledge of the idiosyncrasies of
each kernel version? This is your idea of an improvement?
I didn't get the above #includes out of a kernel tarball, I got them from
Mazur's cleaned up header files. He only included things in userspace that
you actually NEED. They're in the standard available #includes on every
Linuxx development system out there, and the _reason_ they are there is to
expose the available kernel API to user space things that need them. Exactly
so we don't need to block copy some random snapshot and try to fix it up with
version specific #ifdefs that don't work.
Perhaps the C library should have some kind of wrapper for this, but it
doesn't. And if you look at mount from util-linux-2.12, the file
mount/my_dev_t.h, which is included from mount/loop.h, is:
> /* silliness to get dev_t defined as the kernel defines it */
> /* glibc uses a different dev_t */
> /* maybe we need __kernel_old_dev_t -- later */
> /* for ancient systems use "unsigned short" */
>
> #include <linux/posix_types.h>
> #define my_dev_t __kernel_dev_t
So they do it too. And they do it because #include linux/version.h plus
#ifdefs is _UGLIER_, and brittle, and there are kernel versions for which it
won't work.
This stuff should be in a header file. If this is the wrong header file, show
me the right one. It should not be block copied into the program in a way
that requires version specific #ifdefs. That's _more_ wrong than including
the header file, and it's causing me at least real problems. This is not
data we made up, and it's not data we should maintain. We just use it to
talk to the darn kernel. If there is a consistent header file that reliably
defines this info on every linux development system ever shipped, and the
alternative is #ifdefs that break...
Why am I wrong?
Rob
More information about the busybox
mailing list