| Summary: | Missing include in TestNetscapePlugIn | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kalev Lember <kalevlember> | ||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | morrita, peter, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Kalev Lember
2012-02-03 11:43:05 PST
Created attachment 125376 [details] Explicitly include unistd.h in TestNetscapePlugIn Fixes build with gcc 4.7. r100432 added the include only for ANDROID; this changes the ifdef to cover all unix platforms. On Fedora 17 the gcc 4.7 compiler no longer implicitly includes unistd.h from standard headers and other unix platforms are likely to get gcc 4.7 as well in the future. Comment on attachment 125376 [details]
Explicitly include unistd.h in TestNetscapePlugIn
Are you sure the Android build defines XP_UNIX?
Adding Peter Beverloo to CC. Peter, could you check please if XP_UNIX gets defined on Android? Comment on attachment 125376 [details]
Explicitly include unistd.h in TestNetscapePlugIn
We can just have both XP_UNIX and ANDROID. That would be intention revealing and barely hurt the code.
I hope we had EWS for Android but currently we don't. Let us be conservative until then.
Created attachment 127925 [details]
Patch v2
(In reply to comment #4) > We can just have both XP_UNIX and ANDROID. That would be intention revealing and barely hurt the code. Fair enough. Uploaded a new patch that does this. Sorry for the late reply, this slipped through! The change works well for Android, SGTM. Only nit would be that the title could be slightly clearer by appending "for GCC 4.7", but I was guilty of not doing that myself in the Android patch as well, so it should be fine :-). Thanks! Comment on attachment 127925 [details]
Patch v2
Perter, thanks for investigation!
@kalevlember could you update the bug summary? I'm happy to land this then.
Oops s/Perter/Peter/. Created attachment 128154 [details]
Patch v3
Thanks! Updated the patch title as requested.
Comment on attachment 128154 [details] Patch v3 Clearing flags on attachment: 128154 Committed r108540: <http://trac.webkit.org/changeset/108540> All reviewed patches have been landed. Closing bug. |