Closed
Bug 631155
Opened 12 years ago
Closed 12 years ago
undefined return value in function '_cairo_surface_wrapper_flush'
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: wolfiR, Assigned: emorley)
References
(Blocks 1 open bug)
Details
(Whiteboard: [build_warning])
Attachments
(1 file, 1 obsolete file)
2.76 KB,
patch
|
emorley
:
review+
|
Details | Diff | Splinter Review |
A patch from bug 612846 introduced the new function _cairo_surface_wrapper_flush which is just: cairo_status_t _cairo_surface_wrapper_flush (cairo_surface_wrapper_t *wrapper) { if (wrapper->target->backend->flush) { return wrapper->target->backend->flush(wrapper->target); } } gcc is rightfully complaining: /usr/src/packages/BUILD/mozilla/gfx/cairo/cairo/src/cairo-surface-wrapper.c: In function '_cairo_surface_wrapper_flush': /usr/src/packages/BUILD/mozilla/gfx/cairo/cairo/src/cairo-surface-wrapper.c:534:1: warning: control reaches end of non-void function cairo-tee-surface.c The function must return a correct cairo_status_t in every case.
Assignee | ||
Comment 1•12 years ago
|
||
I presume you mean just adding something like: |return CAIRO_STATUS_ZZZZZZZ;| ...after the if block here: http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-surface-wrapper.c#719 What status code should it be returning out of the list here? http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo.h#289 Presumably one of the error codes since |if (wrapper->target->backend->flush)| would have failed? Thanks!
Assignee | ||
Comment 2•12 years ago
|
||
Jeff, could you take a look at comment 1 for me please. Thanks! :-)
Comment 3•12 years ago
|
||
Ping.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → bmo
Status: NEW → ASSIGNED
Comment 4•12 years ago
|
||
I think returning CAIRO_STATUS_SUCCESS makes the most sense here. If we don't have a flush method we don't need to worry about flushing.
Assignee | ||
Comment 5•12 years ago
|
||
Makes sense to me :-) Removes tab whilst I'm there.
Attachment #544555 -
Flags: review?(jmuizelaar)
Comment 6•12 years ago
|
||
Comment on attachment 544555 [details] [diff] [review] Patch v1 Not sure if the indentation change is intentional. Please also include the patch in the cairo directory and update the README there.
Attachment #544555 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 7•12 years ago
|
||
The indentation change was intentional (to make it match the lines before/after), however, I've reverted it now to reduce the number of upstream lines changed. Only other changes are the addition of the patch to the cairo patches collection & the mention of it in the README there. Carrying forwards r+.
Attachment #544555 -
Attachment is obsolete: true
Attachment #544766 -
Flags: review+
Assignee | ||
Comment 8•12 years ago
|
||
Passed try: http://dev.philringnalda.com/tbpl/?tree=Try&rev=7e2dec83b318
Keywords: checkin-needed
Assignee | ||
Updated•12 years ago
|
Blocks: buildwarning
Whiteboard: [build_warning]
![]() |
||
Comment 9•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/a02ef4cdf6b2
Flags: in-testsuite-
Keywords: checkin-needed
Comment 10•12 years ago
|
||
Merged: http://hg.mozilla.org/mozilla-central/rev/a02ef4cdf6b2
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in
before you can comment on or make changes to this bug.
Description
•