#258 fix build error on musl libc (linux/cdrom.h namespace clash).

Unstable_(example)
open
nobody
None
1
2013-04-29
2013-04-13
rofl0r
No

fix build error on musl libc (linux/cdrom.h namespace clash).
since the libc defines some of the same symbol as the kernel headers, there is a clash.
using linux headers directly in userspace is almost always a bad idea, so i ripped the handful of required declarations out of the cdrom.h header and supplied it in a standalone file.
this also removes the need of having the kernel headers installed at all.

1 Attachments

Discussion

  • rofl0r
    rofl0r
    2013-04-13

    inline version of the patch for quick review without dealing with downloads to harddisk, browsing around, and opening files in an editor:

    --- dosbox-0.74.org/src/dos/cdrom_ioctl_linux.cpp 2013-04-13 16:06:02.070000003 +0000
    +++ dosbox-0.74/src/dos/cdrom_ioctl_linux.cpp 2013-04-13 16:06:42.168000002 +0000
    @@ -24,7 +24,7 @@
    #if defined (LINUX)
    #include <fcntl.h>
    #include <unistd.h>
    -#include <linux cdrom.h="">
    +#include "linux_cdrom.h"
    #include <sys ioctl.h="">
    #include <sys stat.h="">
    #include <sys types.h="">
    --- /dev/null 2012-12-26 22:18:45.184000007 +0000
    +++ dosbox-0.74/src/dos/linux_cdrom.h 2013-04-13 16:30:39.779000003 +0000
    @@ -0,0 +1,20 @@
    +#ifndef LINUX_CDROM_H
    +#define LINUX_CDROM_H
    +
    +#define CD_FRAMESIZE 2048
    +#define CD_FRAMESIZE_RAW 2352
    +#define CDROM_GET_MCN 0x5311
    +#define CDROMREADRAW 0x5314
    +
    +struct cdrom_read {
    + int cdread_lba;
    + char *cdread_bufaddr;
    + int cdread_buflen;
    +};
    +
    +struct cdrom_mcn {
    + unsigned char medium_catalog_number[14];
    +};
    +
    +#endif
    +

     
  • rofl0r
    rofl0r
    2013-04-13

    note that in the above text the "lower than" character got mistreated
    so it basically removes include linux/cdrom.h and instead provides linux_cdrom.h

     
  • Peter Veenstra
    Peter Veenstra
    2013-04-15

    I am confused as to why dosbox would need patching ?
    Isn't this just a bug in musl libc ?

    DOSBox interfaces with devices directly, so using kernel headers makes sense to me.
    If musl libc can't handle a kernel header, than that is their problem imho.

    Feel free to point out some reasons to convince me otherwise.

     
  • rofl0r
    rofl0r
    2013-04-21

    is there something that disturbs you about the patch ?

    basically it only takes out 4 magic numbers (from iso9660 standard) and 2 structs (kernel abi) (note: none of these will ever change), and removes the need to the have the linux headers installed.

    isn't that a good thing?

    the reason it works with glibc is that glibc deliberately includes the kernel headers for their own structs. so in order to compile gibc you have to have one of the few working kernel-header versions installed (in most linux releases the userspace kernel headers are broken).
    musl otoh strives to be a complete implementation without any dependencies, thus it has to declare the types and macros dictated by the C standard and POSIX.
    it's not the libc's job to work around incompatibilites with kernel headers (you simply should not use them from userspace, but if you do your translation unit must not be including other userspace headers.

     
  • rofl0r
    rofl0r
    2013-04-29

    nevermind, the bug was actually in the version of kernel headers i got installed (3.8.6). this bug does not appear with over kernel header version.
    http://sprunge.us/BDCF

    but still, since userspace kernel headers are often broken (as in my case), you may want to consider applying the patch none the less.