Bug 154718 - kdemultimedia doesn't build on NetBSD
Summary: kdemultimedia doesn't build on NetBSD
Status: RESOLVED FIXED
Alias: None
Product: kdemultimedia
Classification: Miscellaneous
Component: general (show other bugs)
Version: unspecified
Platform: NetBSD pkgsrc NetBSD
: NOR normal
Target Milestone: ---
Assignee: Multimedia Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-27 22:23 UTC by Mark Davies
Modified: 2008-01-04 00:12 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
treat NetBSD like FreeBSD (354 bytes, patch)
2007-12-27 22:25 UTC, Mark Davies
Details
NetBSD uses statvfs() (1.02 KB, patch)
2007-12-27 22:27 UTC, Mark Davies
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Davies 2007-12-27 22:23:54 UTC
Version:            (using KDE KDE 3.97.0)
Installed from:    NetBSD pkgsrc
Compiler:          gcc4 
OS:                NetBSD

wmlib has a couple of issues that stop it compiling on NetBSD.
Patches attached to fix.
Comment 1 Mark Davies 2007-12-27 22:25:28 UTC
Created attachment 22716 [details]
treat NetBSD like FreeBSD
Comment 2 Mark Davies 2007-12-27 22:27:50 UTC
Created attachment 22717 [details]
NetBSD uses statvfs()

Probably should use a CMake test for statvfs() but I haven't done that yet.
Comment 3 Michael Pyne 2008-01-03 06:11:43 UTC
Sorry about the long wait but is fstatfs simply unavailable in NetBSD (i.e. that won't work at all for NetBSD <2.99.9) or is this simply a more preferred API?
Comment 4 Mark Davies 2008-01-03 09:08:02 UTC
Older NetBSD's have statfs().  Newer ones have statvfs().  None have both.
From the manual page:
HISTORY
     The statvfs(), statvfs1(), fstatvfs(), and fstatvfs1() functions first
     appeared in NetBSD 3.0 to replace the statfs() family of functions which
     first appeared in 4.4BSD.

kdebase/workspace/kcontrol/infocenter/info/info_netbsd.cpp and kdebase/workspace/plasma/dataengines/soliddevice/soliddeviceengine.cpp have examples of choosing one or the other based on a CMake test.

cheers
mark
Comment 5 Michael Pyne 2008-01-03 10:06:01 UTC
I agree with the cdda.c patch

As far as plat_freebsd.c I don't think it requires a CMake check as it's something that gcc can just as easily check.  I would remove the _H from HAVE_SYS_STATVFS_H as you're looking for the function, not the header (unless you implement a CMake check looking for the header).

I think it would be cleaner to do something along the lines of

#ifdef HAVE_SYS_STATVFS

#define stat_fn fstatvfs
#define statvfs_t struct statvfs

#else

#define stat_fn fstatfs
#define statvfs_t struct statfs

#endif

and then use the type and function in gen_eject() but I suppose it's ugly macros either way so take your pick.  I'll agree with the patch either version (but man this needs to get ported to Solid or something! ;)

Mark, do you have an svn account or does someone else need to commit?
Comment 6 Mark Davies 2008-01-03 11:36:58 UTC
I agree the define should not be HAVE_SYS_STATVFS_H but rather HAVE_STATVFS (drop the SYS as well).  The original dated from an old intention to integrate this with the then kde3 build infrastructure that never happened.

I don't have an svn account so someone else will have to commit it.

I don't have a preference for whether to do it as per my patch or Michael's suggested alternative.

Note that the minimal patch would be just adding:

#ifdef __NetBSD__
# include <sys/param.h>
# if __NetBSD_Version__ >= 299000900     /* 2.99.9 */
#  define statfs statvfs
#  define fstatfs fstatvfs
# endif
#endif

Comment 7 Christian Esken 2008-01-03 21:42:27 UTC
Hello,

I can apply the patch. But which one? If it is from comment 5 or comment 6, please provide a diff, as I don't want to guess where to add those lines.
Comment 8 Michael Pyne 2008-01-04 00:12:48 UTC
I've committed a fix for this as of revision 756952.

Here's the commit message and diff:

----------------
Fix bug 154718 (kdemultimedia doesn't build on NetBSD) with patches submitted by Mark Davies and tweaked a bit by myself.

Ended up simply replacing the 10 line preprocessor crap about htonl with a
simple #include <arpa/inet.h> which the Linux, NetBSD, and FreeBSD man pages
available to me all agree is where htonl is defined.

BUG:154718

-----------------
Index: plat_freebsd.c
===================================================================
--- plat_freebsd.c      (revision 756951)
+++ plat_freebsd.c      (revision 756952)
@@ -51,17 +51,31 @@
 # include <sys/scsiio.h>
 # include "/sys/scsi/scsi_all.h"
 # include "/sys/scsi/scsi_cd.h"
+
 #elif defined(__NetBSD__)
-#include <sys/scsiio.h>
-#include <dev/scsipi/scsipi_cd.h>
-#else
+
+# include <sys/scsiio.h>
+# include <dev/scsipi/scsipi_cd.h>
+
+/* Fix http://bugs.kde.org/show_bug.cgi?id=154718 by wrapping calls to removed
+ * functions with their replacements in NetBSD 3.0
+ */
+# if __NetBSD_Version__ >= 299000900    /* 2.99.9 */
+#  include <sys/statvfs.h>
+#  define statfs statvfs
+#  define fstatfs fstatvfs
+# endif
+
+#else /* Not OpenBSD, not NetBSD, therefore (probably) FreeBSD */
+
 # define LEFT_PORT 0
 # define RIGHT_PORT 1
 # if __FreeBSD_version < 300000
 #  include <scsi.h>
 # endif
-#endif

+#endif /* if defined(__OpenBSD__) */
+
 #include "include/wm_struct.h"
 #include "include/wm_platform.h"
 #include "include/wm_cdrom.h"
@@ -507,5 +521,5 @@
        return 0;
 }

-#endif
+#endif /* If FreeBSD or NetBSD */

Index: cdda.c
===================================================================
--- cdda.c      (revision 756951)
+++ cdda.c      (revision 756952)
@@ -20,6 +20,7 @@
 #include <string.h>
 #include <sys/poll.h>
 #include <sys/wait.h>
+#include <arpa/inet.h> /* For htonl(3) */
 #include <stdio.h>
 #include <unistd.h>
 #include "include/wm_config.h"
@@ -75,20 +76,6 @@
        u_32 channels;
 };

-/* had to change #ifdef to #if   -> see wm_cdda.h */
-#ifdef __FreeBSD__
-/* Phungus not with htonl on FreeBSD */
-#include <sys/param.h>
-#else
-#if WM_BIG_ENDIAN
-# ifndef htonl
-#  define htonl(x) (x)
-# endif
-#else
-extern unsigned long htonl(unsigned long);
-#endif
-#endif
-
 static int cdda_status(struct wm_drive *d, int oldmode,
   int *mode, int *frame, int *track, int *ind)
 {