The default bug view has changed. See this FAQ.

undefined return value in function '_cairo_surface_wrapper_flush'

RESOLVED FIXED in mozilla8

Status

()

Core
Graphics
--
major
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: wolfiR, Assigned: emorley)

Tracking

(Blocks: 1 bug)

Trunk
mozilla8
All
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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

6 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

6 years ago
Jeff, could you take a look at comment 1 for me please.

Thanks! :-)

Comment 3

6 years ago
Ping.
(Assignee)

Updated

6 years ago
Assignee: nobody → bmo
Status: NEW → ASSIGNED
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

6 years ago
Created attachment 544555 [details] [diff] [review]
Patch v1

Makes sense to me :-)

Removes tab whilst I'm there.
Attachment #544555 - Flags: review?(jmuizelaar)
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

6 years ago
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+.
Attachment #544555 - Attachment is obsolete: true
Attachment #544766 - Flags: review+
(Assignee)

Comment 8

6 years ago
Passed try:
http://dev.philringnalda.com/tbpl/?tree=Try&rev=7e2dec83b318
Keywords: checkin-needed
(Assignee)

Updated

6 years ago
Blocks: 187528
Whiteboard: [build_warning]
http://hg.mozilla.org/integration/mozilla-inbound/rev/a02ef4cdf6b2
Flags: in-testsuite-
Keywords: checkin-needed
Merged:
http://hg.mozilla.org/mozilla-central/rev/a02ef4cdf6b2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.