After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776323 - Various warning and Coverity fixes
Various warning and Coverity fixes
Status: RESOLVED FIXED
Product: libgxps
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: libgxps maintainers
libgxps maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-20 19:51 UTC by Philip Withnall
Modified: 2017-01-29 07:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
lib: Fix uninitialised variables (1.57 KB, patch)
2016-12-20 19:51 UTC, Philip Withnall
committed Details | Review
lib: Prevent created property being set when content-type is set (942 bytes, patch)
2016-12-20 19:52 UTC, Philip Withnall
committed Details | Review
lib: Document explicit fall-through case in glyph parser (917 bytes, patch)
2016-12-20 19:52 UTC, Philip Withnall
committed Details | Review
lib: Remove redundant NULL check (1.30 KB, patch)
2016-12-20 19:52 UTC, Philip Withnall
reviewed Details | Review
lib: Ignore a warning about a deprecated libarchive function (1.15 KB, patch)
2016-12-20 19:52 UTC, Philip Withnall
committed Details | Review
lib: Fix potential NULL pointer dereference (1.53 KB, patch)
2016-12-22 15:00 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2016-12-20 19:51:55 UTC
Fixes from running Coverity on libgxps. These should all be explained in the commit messages, but please let me know if anything is unclear.
Comment 1 Philip Withnall 2016-12-20 19:51:59 UTC
Created attachment 342280 [details] [review]
lib: Fix uninitialised variables

These could have been used later in the code if the relevant keys were
not present in the XPS file to set their values.

Coverity IDs: 1391239, 1391240
Comment 2 Philip Withnall 2016-12-20 19:52:04 UTC
Created attachment 342281 [details] [review]
lib: Prevent created property being set when content-type is set

There was a missing break statement.

Coverity ID: 1391236
Comment 3 Philip Withnall 2016-12-20 19:52:09 UTC
Created attachment 342282 [details] [review]
lib: Document explicit fall-through case in glyph parser

Make it clear that this is not an accidentally missing break statement.

Coverity ID: 1391237
Comment 4 Philip Withnall 2016-12-20 19:52:15 UTC
Created attachment 342283 [details] [review]
lib: Remove redundant NULL check

utf8 is dereferenced earlier in the function, so if it’s NULL then we
will have already crashed. Rearrange the function to add an explicit
non-NULL check.

Coverity ID: 1391238
Comment 5 Philip Withnall 2016-12-20 19:52:19 UTC
Created attachment 342284 [details] [review]
lib: Ignore a warning about a deprecated libarchive function

archive_read_finish() is deprecated in favour of archive_read_free() in
libarchive version 3.0; but was not deprecated in version 2.0, which we
need to continue to support.

See http://manpages.org/archive_read_finish/3
Comment 6 Carlos Garcia Campos 2016-12-21 16:42:00 UTC
Comment on attachment 342281 [details] [review]
lib: Prevent created property being set when content-type is set

Oops
Comment 7 Carlos Garcia Campos 2016-12-21 16:50:04 UTC
Review of attachment 342283 [details] [review]:

::: libgxps/gxps-glyphs.c
@@ +205,3 @@
+        g_assert (utf8 != NULL);
+
+        utf8_len = g_utf8_next_char (utf8) - utf8;

This is also useless if *utf8 == '\0'. I think we could leave the current early return and simply use g_utf8_next_char (utf8) - utf8 in the  cairo_scaled_font_text_to_glyphs call instead of the utf8_len variable.
Comment 8 Philip Withnall 2016-12-22 14:54:51 UTC
Attachment 342280 [details] pushed as 86d1e66 - lib: Fix uninitialised variables
Attachment 342281 [details] pushed as ae5f6a3 - lib: Prevent created property being set when content-type is set
Attachment 342282 [details] pushed as 5444885 - lib: Document explicit fall-through case in glyph parser
Attachment 342284 [details] pushed as b8d9bf2 - lib: Ignore a warning about a deprecated libarchive function
Comment 9 Philip Withnall 2016-12-22 15:00:03 UTC
Created attachment 342390 [details] [review]
lib: Fix potential NULL pointer dereference

utf8 is dereferenced earlier in the function than the NULL check, so
if it is NULL, this will crash. Move the length calculation lower down
to avoid this.

Coverity ID: 1391238
Comment 10 Carlos Garcia Campos 2017-01-29 07:46:17 UTC
Comment on attachment 342390 [details] [review]
lib: Fix potential NULL pointer dereference

I had forgotten this, pushed now.