Re: clang6: x11/kde4/libs: some tentative fixes

[Christian Weisgerber]

Ingo Schwarze:

> 
> > However, the dnssd/servicemodel.h patch changes an ABI.
> 
> Indeed, you are right.  That definitely requires a major bump,
> which wasn't done earlier because it did not build yet.
> 
> So, here is the hopefully final patch.  I tested the full
> build again, and it succeeded.

ok

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: clang6: x11/kde4/libs: some tentative fixes

[Ingo Schwarze]

Hi Christian,

Christian Weisgerber wrote on Thu, Apr 19, 2018 at 06:20:27AM +0200:

> However, the dnssd/servicemodel.h patch changes an ABI.

Indeed, you are right.  That definitely requires a major bump,
which wasn't done earlier because it did not build yet.

So, here is the hopefully final patch.  I tested the full
build again, and it succeeded.

OK?
  Ingo


Index: Makefile
===
RCS file: /cvs/ports/x11/kde4/libs/Makefile,v
retrieving revision 1.82
diff -u -p -r1.82 Makefile
--- Makefile14 Nov 2017 20:09:11 -  1.82
+++ Makefile19 Apr 2018 14:51:37 -
@@ -12,7 +12,7 @@ PKGNAME-langlist =kde4-langlist-$V
 PKG_ARCH-en_US =   *
 PKG_ARCH-langlist =*
 PKGSPEC-main = kdelibs-${MODKDE4_SPEC}
-REVISION-main =11
+REVISION-main =12
 REVISION-en_US =   0
 REVISION-langlist =0
 
@@ -25,8 +25,8 @@ SHARED_LIBS +=kdecore  50.2
 SHARED_LIBS += kdefakes 50.2  # .5.0
 SHARED_LIBS += kdesu50.2  # .5.0
 SHARED_LIBS += kdeui50.2  # .5.0
-SHARED_LIBS += kdnssd   50.2  # .2.0
-SHARED_LIBS += khtml50.2  # .5.0
+SHARED_LIBS += kdnssd   51.0  # .2.0
+SHARED_LIBS += khtml51.0  # .5.0
 SHARED_LIBS += kimproxy 50.2  # .4.0
 SHARED_LIBS += kio  50.3  # .5.0
 SHARED_LIBS += kjs  50.2  # .2.0
Index: patches/patch-khtml_dom_dom2_traversal_h
===
RCS file: patches/patch-khtml_dom_dom2_traversal_h
diff -N patches/patch-khtml_dom_dom2_traversal_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-khtml_dom_dom2_traversal_h19 Apr 2018 14:51:37 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: khtml/dom/dom2_traversal.h
+--- khtml/dom/dom2_traversal.h.orig
 khtml/dom/dom2_traversal.h
+@@ -214,7 +214,7 @@ class KHTML_EXPORT NodeFilter (public)
+  *
+  */
+ enum ShowCode {
+-SHOW_ALL   = 0x,
++SHOW_ALL   = 0x7FFF,
+ SHOW_ELEMENT   = 0x0001,
+ SHOW_ATTRIBUTE = 0x0002,
+ SHOW_TEXT  = 0x0004,

Re: clang6: x11/kde4/libs: some tentative fixes

[Christian Weisgerber]

Ingo Schwarze:

> I don't think bumping the shared object major is required.  The
> patch only changes *unused* bits in a constant that confused the
> type checking of the compiler.  Even if a program passes the other
> value to a library or vice versa, everything should still work as
> expected.

However, the dnssd/servicemodel.h patch changes an ABI.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: clang6: x11/kde4/libs: some tentative fixes

[Ingo Schwarze]

Hi,

Theo de Raadt wrote on Wed, Apr 18, 2018 at 07:03:55PM -0600:
> Ingo Schwarze  wrote:

>> I don't think bumping the shared object major is required.  The
>> patch only changes *unused* bits in a constant that confused the
>> type checking of the compiler.  Even if a program passes the other
>> value to a library or vice versa, everything should still work as
>> expected.

> I urge you to always crank numbers, because it is free.
> 
> What isn't free, is some reasoning which things it isn't required,
> and then 40 years in the wilderness.
> 
> About 20 years ago, there was a very small #define change that made a
> pile of libraries incompatible in a subtle way, and people were ready to
> kill each other.  If the libraries had just been cranked (in the same
> way we aggressively crank libc), we wouldn't even have noticed the
> problem.

You are right, and in principle i know, sorry for being lazy.
I actually *tried* cranking the number, and it blew up somehow
during packaging because i must have done something wrong on the
first try, i don't know what.  So i thought i could get the patch
out earlier in a situation where the bump isn't really mandatory.

Anyway, now i cleaned out everything and rebuilt everything
again from scratch (kdelibs take some time building...)
Little surprise: building and packaging works once you do it right.

So here is the safer patch with the bump.

Yours,
  Ingo


Index: Makefile
===
RCS file: /cvs/ports/x11/kde4/libs/Makefile,v
retrieving revision 1.82
diff -u -p -r1.82 Makefile
--- Makefile14 Nov 2017 20:09:11 -  1.82
+++ Makefile19 Apr 2018 02:29:27 -
@@ -12,7 +12,7 @@ PKGNAME-langlist =kde4-langlist-$V
 PKG_ARCH-en_US =   *
 PKG_ARCH-langlist =*
 PKGSPEC-main = kdelibs-${MODKDE4_SPEC}
-REVISION-main =11
+REVISION-main =12
 REVISION-en_US =   0
 REVISION-langlist =0
 
@@ -26,7 +26,7 @@ SHARED_LIBS +=kdefakes 50.2
 SHARED_LIBS += kdesu50.2  # .5.0
 SHARED_LIBS += kdeui50.2  # .5.0
 SHARED_LIBS += kdnssd   50.2  # .2.0
-SHARED_LIBS += khtml50.2  # .5.0
+SHARED_LIBS += khtml51.0  # .5.0
 SHARED_LIBS += kimproxy 50.2  # .4.0
 SHARED_LIBS += kio  50.3  # .5.0
 SHARED_LIBS += kjs  50.2  # .2.0
Index: patches/patch-khtml_dom_dom2_traversal_h
===
RCS file: patches/patch-khtml_dom_dom2_traversal_h
diff -N patches/patch-khtml_dom_dom2_traversal_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-khtml_dom_dom2_traversal_h19 Apr 2018 02:29:27 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: khtml/dom/dom2_traversal.h
+--- khtml/dom/dom2_traversal.h.orig
 khtml/dom/dom2_traversal.h
+@@ -214,7 +214,7 @@ class KHTML_EXPORT NodeFilter (public)
+  *
+  */
+ enum ShowCode {
+-SHOW_ALL   = 0x,
++SHOW_ALL   = 0x7FFF,
+ SHOW_ELEMENT   = 0x0001,
+ SHOW_ATTRIBUTE = 0x0002,
+ SHOW_TEXT  = 0x0004,

Re: clang6: x11/kde4/libs: some tentative fixes

[Theo de Raadt]

Ingo Schwarze  wrote:
> I don't think bumping the shared object major is required.  The
> patch only changes *unused* bits in a constant that confused the
> type checking of the compiler.  Even if a program passes the other
> value to a library or vice versa, everything should still work as
> expected.

I urge you to always crank numbers, because it is free.

What isn't free, is some reasoning which things it isn't required,
and then 40 years in the wilderness.

About 20 years ago, there was a very small #define change that made a
pile of libraries incompatible in a subtle way, and people were ready to
kill each other.  If the libraries had just been cranked (in the same
way we aggressively crank libc), we wouldn't even have noticed the
problem.

Re: clang6: x11/kde4/libs: some tentative fixes

[Ingo Schwarze]

Hi Christian,

Christian Weisgerber wrote on Tue, Apr 17, 2018 at 06:52:14PM +0200:

> Here are three further tentative fixes for x11/kde4/libs.  There
> are additional problems still, but I'm going to take a break now.

Actually, you did *all* the work required and merely missed the fact
that you were successful.

With just one tiny patch that i already committed to kdelibs-3,
kdelibs-4 now builds, too.  Again, i did not test at run time but
as jasper@ agreed, that will get tested when people fix the dozens
of large applications using this.  It may be helpful to get this
in soon, after the delay we already had and in pleasant anticipation
of the imminent strike...  err, i intended to say: hackathon.

I don't think bumping the shared object major is required.  The
patch only changes *unused* bits in a constant that confused the
type checking of the compiler.  Even if a program passes the other
value to a library or vice versa, everything should still work as
expected.

OK?
  Ingo


Index: Makefile
===
RCS file: /cvs/ports/x11/kde4/libs/Makefile,v
retrieving revision 1.82
diff -u -p -r1.82 Makefile
--- Makefile14 Nov 2017 20:09:11 -  1.82
+++ Makefile19 Apr 2018 00:43:48 -
@@ -12,7 +12,7 @@ PKGNAME-langlist =kde4-langlist-$V
 PKG_ARCH-en_US =   *
 PKG_ARCH-langlist =*
 PKGSPEC-main = kdelibs-${MODKDE4_SPEC}
-REVISION-main =11
+REVISION-main =12
 REVISION-en_US =   0
 REVISION-langlist =0
 
Index: patches/patch-khtml_dom_dom2_traversal_h
===
RCS file: patches/patch-khtml_dom_dom2_traversal_h
diff -N patches/patch-khtml_dom_dom2_traversal_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-khtml_dom_dom2_traversal_h19 Apr 2018 00:43:48 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: khtml/dom/dom2_traversal.h
+--- khtml/dom/dom2_traversal.h.orig
 khtml/dom/dom2_traversal.h
+@@ -214,7 +214,7 @@ class KHTML_EXPORT NodeFilter (public)
+  *
+  */
+ enum ShowCode {
+-SHOW_ALL   = 0x,
++SHOW_ALL   = 0x7FFF,
+ SHOW_ELEMENT   = 0x0001,
+ SHOW_ATTRIBUTE = 0x0002,
+ SHOW_TEXT  = 0x0004,

Re: clang6: x11/kde4/libs: some tentative fixes

[Ingo Schwarze]

Hi Christian,

Christian Weisgerber wrote on Tue, Apr 17, 2018 at 06:52:14PM +0200:

> Here are three further tentative fixes for x11/kde4/libs.
> There are additional problems still, but I'm going to take
> a break now.

I checked that your patches are indeed needed, that they indeed
solve the respective problems, and that the code change and your
explanation makes sense.

I think you should commit these, it gets us closer to getting
the libs to build.

> Some comments:
> 
> * dnssd/servicemodel.h:
>   The value of ServicePtrRole appears to be random number that would
>   be unlikely to collide with the values already used in Qt::ItemDataRole.
>   It needs to fit into an int, so I dropped the top bit.  Maybe a
>   cast in servicemodel.cpp would be better, dunno.

OK.

> * kdeui/windowmanagement/netwm.cpp:
>   None is normally defined to 0L by the X11 headers, but
>   kdeui/util/fixx11h.h performs some special magic to turn it into
>   a const integral variable with value 0, which is too far off from
>   a null pointer for clang6.  Since the intend is to compare against
>   a null pointer, just use NULL.

OK.

> * khtml/misc/AtomicString.cpp:
>   String length is unsigned, but AtomicString::add() takes an int
>   parameter.  In principle, it would be better to change the parameter
>   to unsigned, but I'm afraid this will just push the problem to
>   any callers.

OK.

Yours,
  Ingo


> Index: patches/patch-dnssd_servicemodel_h
> ===
> RCS file: patches/patch-dnssd_servicemodel_h
> diff -N patches/patch-dnssd_servicemodel_h
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-dnssd_servicemodel_h17 Apr 2018 16:39:05 -
> @@ -0,0 +1,14 @@
> +$OpenBSD$
> +
> +Index: dnssd/servicemodel.h
> +--- dnssd/servicemodel.h.orig
>  dnssd/servicemodel.h
> +@@ -71,7 +71,7 @@ class KDNSSD_EXPORT ServiceModel : public QAbstractIte
> + 
> + /** The additional data roles provided by this model */
> + enum AdditionalRoles {
> +-ServicePtrRole = 0xA06519DE  ///< gets a RemoteService::Ptr for 
> the service
> ++ServicePtrRole = 0x206519DE  ///< gets a RemoteService::Ptr for 
> the service
> + };
> + 
> + /**
> Index: patches/patch-kdeui_windowmanagement_netwm_cpp
> ===
> RCS file: patches/patch-kdeui_windowmanagement_netwm_cpp
> diff -N patches/patch-kdeui_windowmanagement_netwm_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-kdeui_windowmanagement_netwm_cpp17 Apr 2018 16:39:05 
> -
> @@ -0,0 +1,14 @@
> +$OpenBSD$
> +
> +Index: kdeui/windowmanagement/netwm.cpp
> +--- kdeui/windowmanagement/netwm.cpp.orig
>  kdeui/windowmanagement/netwm.cpp
> +@@ -4368,7 +4368,7 @@ void NETWinInfo::update(const unsigned long dirty_prop
> + if (XGetWindowProperty(p->display, p->window, 
> kde_net_wm_block_compositing, 0l,
> +1, False, XA_STRING, _ret,
> +_ret, _ret, , 
> _ret) == Success) {
> +-p->blockCompositing = (data_ret != None);
> ++p->blockCompositing = (data_ret != NULL);
> + if (data_ret)  // stupid question to everyone - since the 
> result is "Success", is this check required?
> + XFree(data_ret);
> + }
> Index: patches/patch-khtml_misc_AtomicString_cpp
> ===
> RCS file: patches/patch-khtml_misc_AtomicString_cpp
> diff -N patches/patch-khtml_misc_AtomicString_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ patches/patch-khtml_misc_AtomicString_cpp 17 Apr 2018 16:39:05 -
> @@ -0,0 +1,23 @@
> +$OpenBSD$
> +
> +Index: khtml/misc/AtomicString.cpp
> +--- khtml/misc/AtomicString.cpp.orig
>  khtml/misc/AtomicString.cpp
> +@@ -160,7 +160,7 @@ DOMStringImpl* AtomicString::add(const QChar* s, int l
> + return DOMStringImpl::empty();
> +
> + init();
> +-UCharBuffer buf = { s, length }; 
> ++UCharBuffer buf = { s, static_cast(length) }; 
> + std::pair::iterator, bool> addResult = 
> stringTable->add(buf);
> + if (!addResult.second)
> + return *addResult.first;
> +@@ -172,7 +172,7 @@ DOMStringImpl* AtomicString::add(const QChar* s)
> + if (!s)
> + return 0;
> + 
> +-int length = 0;
> ++unsigned length = 0;
> + while (s[length] != QChar(0))
> + length++;
> + 

clang6: x11/kde4/libs: some tentative fixes

[Christian Weisgerber]

Here are three further tentative fixes for x11/kde4/libs.  There are
additional problems still, but I'm going to take a break now.

Some comments:

* dnssd/servicemodel.h:
  The value of ServicePtrRole appears to be random number that would
  be unlikely to collide with the values already used in Qt::ItemDataRole.
  It needs to fit into an int, so I dropped the top bit.  Maybe a
  cast in servicemodel.cpp would be better, dunno.

* kdeui/windowmanagement/netwm.cpp:
  None is normally defined to 0L by the X11 headers, but
  kdeui/util/fixx11h.h performs some special magic to turn it into
  a const integral variable with value 0, which is too far off from
  a null pointer for clang6.  Since the intend is to compare against
  a null pointer, just use NULL.

* khtml/misc/AtomicString.cpp:
  String length is unsigned, but AtomicString::add() takes an int
  parameter.  In principle, it would be better to change the parameter
  to unsigned, but I'm afraid this will just push the problem to
  any callers.

Index: patches/patch-dnssd_servicemodel_h
===
RCS file: patches/patch-dnssd_servicemodel_h
diff -N patches/patch-dnssd_servicemodel_h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-dnssd_servicemodel_h  17 Apr 2018 16:39:05 -
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: dnssd/servicemodel.h
+--- dnssd/servicemodel.h.orig
 dnssd/servicemodel.h
+@@ -71,7 +71,7 @@ class KDNSSD_EXPORT ServiceModel : public QAbstractIte
+ 
+   /** The additional data roles provided by this model */
+   enum AdditionalRoles {
+-  ServicePtrRole = 0xA06519DE  ///< gets a RemoteService::Ptr for 
the service
++  ServicePtrRole = 0x206519DE  ///< gets a RemoteService::Ptr for 
the service
+   };
+ 
+   /**
Index: patches/patch-kdeui_windowmanagement_netwm_cpp
===
RCS file: patches/patch-kdeui_windowmanagement_netwm_cpp
diff -N patches/patch-kdeui_windowmanagement_netwm_cpp
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-kdeui_windowmanagement_netwm_cpp  17 Apr 2018 16:39:05 
-
@@ -0,0 +1,14 @@
+$OpenBSD$
+
+Index: kdeui/windowmanagement/netwm.cpp
+--- kdeui/windowmanagement/netwm.cpp.orig
 kdeui/windowmanagement/netwm.cpp
+@@ -4368,7 +4368,7 @@ void NETWinInfo::update(const unsigned long dirty_prop
+ if (XGetWindowProperty(p->display, p->window, 
kde_net_wm_block_compositing, 0l,
+1, False, XA_STRING, _ret,
+_ret, _ret, , _ret) 
== Success) {
+-p->blockCompositing = (data_ret != None);
++p->blockCompositing = (data_ret != NULL);
+ if (data_ret)  // stupid question to everyone - since the result 
is "Success", is this check required?
+ XFree(data_ret);
+ }
Index: patches/patch-khtml_misc_AtomicString_cpp
===
RCS file: patches/patch-khtml_misc_AtomicString_cpp
diff -N patches/patch-khtml_misc_AtomicString_cpp
--- /dev/null   1 Jan 1970 00:00:00 -
+++ patches/patch-khtml_misc_AtomicString_cpp   17 Apr 2018 16:39:05 -
@@ -0,0 +1,23 @@
+$OpenBSD$
+
+Index: khtml/misc/AtomicString.cpp
+--- khtml/misc/AtomicString.cpp.orig
 khtml/misc/AtomicString.cpp
+@@ -160,7 +160,7 @@ DOMStringImpl* AtomicString::add(const QChar* s, int l
+ return DOMStringImpl::empty();
+
+ init();
+-UCharBuffer buf = { s, length }; 
++UCharBuffer buf = { s, static_cast(length) }; 
+ std::pair::iterator, bool> addResult = 
stringTable->add(buf);
+ if (!addResult.second)
+ return *addResult.first;
+@@ -172,7 +172,7 @@ DOMStringImpl* AtomicString::add(const QChar* s)
+ if (!s)
+ return 0;
+ 
+-int length = 0;
++unsigned length = 0;
+ while (s[length] != QChar(0))
+ length++;
+ 
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de