[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