Bug 53766 - [Qt] Compilation error on OpenBSD
Summary: [Qt] Compilation error on OpenBSD
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P5 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2011-02-04 07:06 PST by Olivier Goffart
Modified: 2011-02-10 21:09 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.69 KB, patch)
2011-02-10 11:59 PST, Patrick R. Gansterer
kling: review-
Details | Formatted Diff | Diff
Alternative Patch (with positiv listing) (1.63 KB, patch)
2011-02-10 13:21 PST, Patrick R. Gansterer
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Goffart 2011-02-04 07:06:20 PST
src/3rdparty/webkit/WebCore/websockets/WebSocketHandshake.cpp:289: error: 'strnstr' was not declared in this scope

That function is apparently not protbale. According to the man page:
"Since the strnstr() function is a FreeBSD specific API, it should only be used when portability is not a concern."
Comment 1 Patrick R. Gansterer 2011-02-08 16:35:46 PST
Please add a OS(OPENBSD) (or whatever) at http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/StringExtras.h?rev=74855#L103
Comment 2 Benjamin Poulain 2011-02-09 03:42:58 PST
(In reply to comment #1)
> Please add a OS(OPENBSD) (or whatever) at http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/StringExtras.h?rev=74855#L103

I would be happy to but I can't make a patch I do not test. Patrick, could you make the patch for OpenBSD and test the build?

Here is some info about making patches: https://trac.webkit.org/wiki/QtWebKitContrib
Ping me on IRC if you have any question.
Comment 3 Patrick R. Gansterer 2011-02-09 03:52:43 PST
(In reply to comment #2)
> I would be happy to but I can't make a patch I do not test.
I have the same "problem" of no OpenBSD installation.

> Patrick, could you make the patch for OpenBSD and test the build?
I'm be willing to create a patch, but Oliver needs to test it first.

Mabye my last comment should be read as:
"Oliver, can you try to add OS(OPENBSD) to StringExtras.h and tell us if that fixes the problem"
Sorry for the unclear comment.
Comment 4 Olivier Goffart 2011-02-10 01:10:52 PST
I confirm that adding || OS(OPENBSD) in StringExtras.h works.

Then webkit compile and i am able to run the Qt demo browser.
Comment 5 Patrick R. Gansterer 2011-02-10 11:59:31 PST
Created attachment 82021 [details]
Patch
Comment 6 Benjamin Poulain 2011-02-10 12:55:27 PST
Comment on attachment 82021 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82021&action=review

> Source/JavaScriptCore/wtf/Platform.h:729
> +#if !defined(HAVE_STRNSTR)
> +#if !COMPILER(MSVC) && !COMPILER(RVCT) && !OS(WINDOWS) && !OS(LINUX) && !OS(SOLARIS) && !OS(OPENBSD)

Shouldn't that be the other way around:
#if OS(MAC) || OS(FREEBSD)
?
Comment 7 Patrick R. Gansterer 2011-02-10 13:21:19 PST
Created attachment 82034 [details]
Alternative Patch (with positiv listing)

View in context: https://bugs.webkit.org/attachment.cgi?id=82021&action=review

>> Source/JavaScriptCore/wtf/Platform.h:729
>> +#if !COMPILER(MSVC) && !COMPILER(RVCT) && !OS(WINDOWS) && !OS(LINUX) && !OS(SOLARIS) && !OS(OPENBSD)
> 
> Shouldn't that be the other way around:
> #if OS(MAC) || OS(FREEBSD)
> ?

Maybe yes, maybe no. ;-)
I don't know if it's defined at AIX, HAIKU, NETBSD, QNX, but changing this to positiv listing is clearly better.

It seams that at least HAIKU needs this change too, so it's ok to take the risk of breaking the build on some "uncommon" platforms IMHO.
Comment 8 Andreas Kling 2011-02-10 13:27:21 PST
Comment on attachment 82034 [details]
Alternative Patch (with positiv listing)

r=me
Comment 9 Andreas Kling 2011-02-10 13:31:29 PST
(In reply to comment #7)
> It seams that at least HAIKU needs this change too, so it's ok to take the risk of breaking the build on some "uncommon" platforms IMHO.

Judging by the qt-haiku port (I had it handy..), Haiku doesn't have strnstr(). http://gitorious.org/webkit/qt-haiku-webkit/commit/b964b5fb44144d48a8bce266db856d3317d12a0f?diffmode=sidebyside
Comment 10 WebKit Commit Bot 2011-02-10 15:26:51 PST
Comment on attachment 82034 [details]
Alternative Patch (with positiv listing)

Clearing flags on attachment: 82034

Committed r78278: <http://trac.webkit.org/changeset/78278>
Comment 11 WebKit Commit Bot 2011-02-10 15:26:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Review Bot 2011-02-10 21:09:55 PST
http://trac.webkit.org/changeset/78278 might have broken GTK Linux 32-bit Release
The following tests are not passing:
http/tests/misc/acid2-pixel.html
http/tests/misc/acid2.html