Closed
Bug 631155
Opened 14 years ago
Closed 14 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•14 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•14 years ago
|
||
Jeff, could you take a look at comment 1 for me please.
Thanks! :-)
Comment 3•14 years ago
|
||
Ping.
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → bmo
Status: NEW → ASSIGNED
Comment 4•14 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•14 years ago
|
||
Makes sense to me :-)
Removes tab whilst I'm there.
Attachment #544555 -
Flags: review?(jmuizelaar)
Comment 6•14 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•14 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•14 years ago
|
||
Keywords: checkin-needed
Assignee | ||
Updated•14 years ago
|
Blocks: buildwarning
Whiteboard: [build_warning]
![]() |
||
Comment 9•14 years ago
|
||
Flags: in-testsuite-
Keywords: checkin-needed
Comment 10•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in
before you can comment on or make changes to this bug.
Description
•