Last Comment Bug 631155 - undefined return value in function '_cairo_surface_wrapper_flush'
: undefined return value in function '_cairo_surface_wrapper_flush'
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Linux
-- major (vote)
: mozilla8
Assigned To: Ed Morley [:emorley]
: Milan Sreckovic [:milan]
Depends on:
Blocks: buildwarning
  Show dependency treegraph
Reported: 2011-02-02 22:47 PST by Wolfgang Rosenauer [:wolfiR]
Modified: 2011-07-12 03:43 PDT (History)
4 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (818 bytes, patch)
2011-07-07 11:27 PDT, Ed Morley [:emorley]
jmuizelaar: review+
Details | Diff | Splinter Review
Patch v1.1 (2.76 KB, patch)
2011-07-08 03:34 PDT, Ed Morley [:emorley]
emorley: review+
Details | Diff | Splinter Review

Description User image Wolfgang Rosenauer [:wolfiR] 2011-02-02 22:47:55 PST
A patch from bug 612846 introduced the new function _cairo_surface_wrapper_flush which is just:
_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

The function must return a correct cairo_status_t in every case.
Comment 1 User image Ed Morley [:emorley] 2011-05-30 07:42:56 PDT
I presume you mean just adding something like:
...after the if block here:

What status code should it be returning out of the list here?

Presumably one of the error codes since |if (wrapper->target->backend->flush)| would have failed?

Comment 2 User image Ed Morley [:emorley] 2011-06-18 17:48:49 PDT
Jeff, could you take a look at comment 1 for me please.

Thanks! :-)
Comment 3 User image Steve Snyder 2011-06-30 11:25:58 PDT
Comment 4 User image Jeff Muizelaar [:jrmuizel] 2011-07-07 11:20:24 PDT
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.
Comment 5 User image Ed Morley [:emorley] 2011-07-07 11:27:32 PDT
Created attachment 544555 [details] [diff] [review]
Patch v1

Makes sense to me :-)

Removes tab whilst I'm there.
Comment 6 User image Jeff Muizelaar [:jrmuizel] 2011-07-07 13:31:55 PDT
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.
Comment 7 User image Ed Morley [:emorley] 2011-07-08 03:34:08 PDT
Created attachment 544766 [details] [diff] [review]
Patch v1.1

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+.
Comment 8 User image Ed Morley [:emorley] 2011-07-09 04:13:57 PDT
Passed try:
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 06:21:59 PDT
Comment 10 User image Mounir Lamouri (:mounir) 2011-07-12 03:43:11 PDT

Note You need to log in before you can comment on or make changes to this bug.