Bug 105151 - [PATCH] Fix various compiler warnings
Summary: [PATCH] Fix various compiler warnings
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: All All
: medium minor
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-18 08:48 UTC by Adam Reichold
Modified: 2018-02-21 20:52 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch to fix compiler warnings (11.57 KB, patch)
2018-02-18 08:48 UTC, Adam Reichold
Details | Splinter Review
Second version of pathc to reduce compiler warnings (11.72 KB, patch)
2018-02-20 07:36 UTC, Adam Reichold
Details | Splinter Review
Patch to reduce compiler warnings (11.54 KB, patch)
2018-02-21 05:41 UTC, Adam Reichold
Details | Splinter Review

Description Adam Reichold 2018-02-18 08:48:44 UTC
Created attachment 137423 [details] [review]
Patch to fix compiler warnings

The attached patch fixes several compiler warnings, mostly in the Qt 5 frontend, to make a full build less noisy and new warnings easier to spot.
Comment 1 Albert Astals Cid 2018-02-18 22:00:19 UTC
Why did you remove the setLinkDestination(movie); call?
Comment 2 Adam Reichold 2018-02-18 22:11:44 UTC
(In reply to Albert Astals Cid from comment #1)
> Why did you remove the setLinkDestination(movie); call?

It was surrounded by #if 0 /* ... */ #endif and hence seemed to be dead code.
Comment 3 Albert Astals Cid 2018-02-19 23:31:33 UTC
-    QCOMPARE( page->search(QString("non-ascii:"), rectLeft, rectTop, rectRight, rectBottom, Poppler::Page::FromTop, Poppler::Page::CaseSensitive), true );
+    QCOMPARE( page->search(QString("non-ascii:"), rectLeft, rectTop, rectRight, rectBottom, Poppler::Page::FromTop, Poppler::Page::IgnoreCase), true );

is wrong?
Comment 4 Adam Reichold 2018-02-20 07:36:21 UTC
Created attachment 137454 [details] [review]
Second version of pathc to reduce compiler warnings
Comment 5 Adam Reichold 2018-02-20 07:38:13 UTC
(In reply to Albert Astals Cid from comment #3)
> -    QCOMPARE( page->search(QString("non-ascii:"), rectLeft, rectTop,
> rectRight, rectBottom, Poppler::Page::FromTop,
> Poppler::Page::CaseSensitive), true );
> +    QCOMPARE( page->search(QString("non-ascii:"), rectLeft, rectTop,
> rectRight, rectBottom, Poppler::Page::FromTop, Poppler::Page::IgnoreCase),
> true );
> 
> is wrong?

Indeed, thanks for spotting it. Fixed that in an updated version of the patch. (It seems that the test is actually not dependent on that flag but still a stronger check in the original version.)
Comment 6 Albert Astals Cid 2018-02-20 23:46:37 UTC
Can you rebase the patch?

tsdgeos@xps:~/devel/poppler:master$ LANG=C git am attachment.cgi\?id\=137454
Applying: Fix buffer size warning due to missing space for null terminator in pdfseparate.
Applying: Fix warnings due to the use of deprecated overloads of Poppler::Page::Search in tests of Qt5 frontend.
error: patch failed: qt5/tests/check_search.cpp:21
error: qt5/tests/check_search.cpp: patch does not apply
Patch failed at 0002 Fix warnings due to the use of deprecated overloads of Poppler::Page::Search in tests of Qt5 frontend.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Seems like the encoding of the file is not utf8 and the special characters are getting confused?
Comment 7 Adam Reichold 2018-02-21 05:40:47 UTC
(In reply to Albert Astals Cid from comment #6)
> Can you rebase the patch?

[...]

> Seems like the encoding of the file is not utf8 and the special characters
> are getting confused?

Yes, one of the edited files was not UTF-8 encoded. The rebase does not change anything, but I think I could have messed up the patch file as I needed to copy-and-paste it from Kate due to a Tumbleweed update killing the kfile kio plug-in and with it all KDE file dialogs. Sorry about that.

I will reattach the newly generated patch file directly.
Comment 8 Adam Reichold 2018-02-21 05:41:20 UTC
Created attachment 137491 [details] [review]
Patch to reduce compiler warnings
Comment 9 Albert Astals Cid 2018-02-21 20:52:08 UTC
to be honest i'm not totally sure i agree with with the last one, but anyway, i've pushed it.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.