Last Comment Bug 631155 - undefined return value in function '_cairo_surface_wrapper_flush'
: undefined return value in function '_cairo_surface_wrapper_flush'
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All Linux
: -- major (vote)
: mozilla8
Assigned To: Ed Morley [:emorley]
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency tree / graph
 
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Attachments
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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Description 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_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.
Comment 1 Ed Morley [:emorley] 2011-05-30 07:42:56 PDT 
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!
Comment 2 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 Steve Snyder 2011-06-30 11:25:58 PDT 
Ping.
Comment 4 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 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 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 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 Ed Morley [:emorley] 2011-07-09 04:13:57 PDT 
Passed try:
http://dev.philringnalda.com/tbpl/?tree=Try&rev=7e2dec83b318
Comment 9 Boris Zbarsky [:bz] (TPAC) 2011-07-11 06:21:59 PDT 
http://hg.mozilla.org/integration/mozilla-inbound/rev/a02ef4cdf6b2
Comment 10 Mounir Lamouri (:mounir) 2011-07-12 03:43:11 PDT 
Merged:
http://hg.mozilla.org/mozilla-central/rev/a02ef4cdf6b2
Note You need to log in before you can comment on or make changes to this bug.